Re: [PATCH 1/2] merge: don't run post-hook logic on --no-verify

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 133831d42fd..fab553e3bc4 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -845,15 +845,18 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>  	struct strbuf msg = STRBUF_INIT;
>  	const char *index_file = get_index_file();
>  
> -	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
> -		abort_commit(remoteheads, NULL);
> -	/*
> -	 * Re-read the index as pre-merge-commit hook could have updated it,
> -	 * and write it out as a tree.  We must do this before we invoke
> -	 * the editor and after we invoke run_status above.
> -	 */
> -	if (hook_exists("pre-merge-commit"))
> -		discard_cache();
> +	if (!no_verify) {
> +		if (run_commit_hook(0 < option_edit, index_file,
> +				    "pre-merge-commit", NULL))
> +			abort_commit(remoteheads, NULL);
> +		/*
> +		 * Re-read the index as pre-merge-commit hook could have updated it,
> +		 * and write it out as a tree.  We must do this before we invoke
> +		 * the editor and after we invoke run_status above.
> +		 */
> +		if (hook_exists("pre-merge-commit"))
> +			discard_cache();
> +	}

The updated code not just is more correct, but it is much easier to
follow.

I wonder if run_commit_hook() can return "I failed to run the hook",
"I ran the hook and the hook failed", "I successfully run the hook",
and "I didn't find the hook", instead of the current "yes/no".  That
would allow us to express this part in an even cleaner way, I would
think.

Looking good.

Thanks.

>  	read_cache_from(index_file);
>  	strbuf_addbuf(&msg, &merge_msg);
>  	if (squash)