Re: [PATCH 2/3] reset: use DIFF_OPT_SET macro to set a diff flag

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 29, 2017, 01:26 UTC
Message-ID: <xmqq1slmbv73.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171027222853.180981-3-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 5 quoted lines

> Instead of explicitly setting the 'DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG'
> flag, use the 'DIFF_OPT_SET' macro.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---

Looks good. It's not like one of 1/3 and 2/3 could be a good idea while the other is not, so it would make a lot more sense to combine them into a single preliminary clean-up patch, though.

In any case, these two are very good clean-up patches, whose value does not diminish even we do not go ahead with 3/3 yet.

Nicely spotted; thanks.

Show 16 quoted lines

>  builtin/reset.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 9cd89b230..ea2fad5a0 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -166,7 +166,7 @@ static int read_from_tree(const struct pathspec *pathspec,
>  	opt.output_format = DIFF_FORMAT_CALLBACK;
>  	opt.format_callback = update_index_from_diff;
>  	opt.format_callback_data = &intent_to_add;
> -	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
> +	DIFF_OPT_SET(&opt, OVERRIDE_SUBMODULE_CONFIG);
>  
>  	if (do_diff_cache(tree_oid, &opt))
>  		return 1;