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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> 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)




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux