Re: [PATCH v5 0/4] githooks.txt improvements + core.hooksDirectory

From: Junio C Hamano <gitster@pobox.com>
Date: May 4, 2016, 22:13 UTC
Message-ID: <xmqqa8k5eakb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1462393128-26762-1-git-send-email-avarab@gmail.com>

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

> For convenience & ease of review a diff between the bits of v4 and v5
> that I changed follows below.

Thanks.

Show 6 quoted lines

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> ...
> +This configuration variable is useful in cases where you'd like to
> +centrally configure your Git hooks instead of configuring them on a
> +per-repository basis, or as a more flexible and centralized
> +alterantive to having an `init.templateDir` where you've changed

"alternative" (can be locally amended).

Show 7 quoted lines

> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> ...
> +Hooks don't need to worry about the user's current working
> +directory. Before Git invokes a hook it changes its working
> +directory. The working directory will be the root of the working tree
> +in a non-bare repository, or in the $GIT_DIR directory in a bare
> +repository.

I suspect that the first sentence does more harm than good. When you say "don't need to worry about", somebody is bound to ask "that is good that I do not need to, but what if I _want_ to know?", and you do not have a good answer to that question.

	Before Git invokes a hook, it changes its working directory
	to either the root of the working tree in a non-bare
	repository, or to the $GIT_DIR in a bare repository.

perhaps?

Show 16 quoted lines

> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> index f2f0aa9..6e4586a 100755
> --- a/t/t1350-config-hooks-path.sh
> +++ b/t/t1350-config-hooks-path.sh
> @@ -7,10 +7,10 @@ test_description='Test the core.hooksPath configuration variable'
>  test_expect_success 'set up a pre-commit hook in core.hooksPath' '
>  	mkdir -p .git/custom-hooks .git/hooks &&
>  	write_script .git/custom-hooks/pre-commit <<-\EOF &&
> -printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +	echo CUSTOM >>actual
>  EOF
>  	write_script .git/hooks/pre-commit <<-\EOF
> -printf "%s" "NORM" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +	echo NORMAL >>actual
>  EOF
>  '

These closing EOF can and should be indented like the body of the here-text, i.e.

	cmd <<-\EOF &&
        ...
        EOF

Show 13 quoted lines

> @@ -24,8 +24,14 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'
>  	test_commit have_custom_hook_abs_path &&
>  	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
>  	test_commit have_custom_hook_abs_path_trailing_slash &&
> -	printf "%s" "NORMCUSTCUSTCUSTCUST" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> -	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
> +	cat >expect <<-\EOF &&
> +	NORMAL
> +	CUSTOM
> +	CUSTOM
> +	CUSTOM
> +	CUSTOM
> +EOF

Likewise.