Re: [PATCH 1/3] githooks.txt: Improve the intro section

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 25, 2016 at 8:23 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason   writes:
>
>> Change the documentation so that:
>>
>>  * We don't talk about "little scripts". Hooks can be as big as you
>>want, and don't have to be scripts, just call them "programs".
>>
>>  * We note what happens with chdir() before a hook is called, nothing
>>documented this explicitly, but the current behavior is
>>predictable. It helps a lot to know what directory these hooks will
>>be executed from.
>>
>>  * We don't make claims about the example hooks which may not be true
>>depending on the configuration of 'init.templateDir'. Clarify that
>>we're talking about the default settings of git-init in those cases,
>>and move some of this documentation into git-init's documentation
>>about the default templates.
>>
>>  * We briefly note in the intro that hooks can get their arguments in
>>various different ways, and that how exactly is described below for
>>each hook.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>  Documentation/git-init.txt |  6 +-
>>  Documentation/githooks.txt | 32 
>>  2 files changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
>> index 8174d27..cf37926 100644
>> --- a/Documentation/git-init.txt
>> +++ b/Documentation/git-init.txt
>> @@ -130,7 +130,11 @@ The template directory will be one of the following (in 
>> order):
>>   - the default template directory: `/usr/share/git-core/templates`.
>>
>>  The default template directory includes some directory structure, suggested
>> -"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see 
>> linkgit:githooks[5]).
>> +"exclude patterns" (see linkgit:gitignore[5]), and example hook files.
>> +
>> +The example are all disabled by default. To enable a hook, rename it
>
> "sample hooks are all disabled" would be better; if for some unknown
> reason you are trying to avoid "sample hooks", "examples are all
> disabled" may be acceptable.
>
>> +by removing its `.sample` suffix. See linkgit:githooks[5] for more
>> +info on hook execution.
>
> Makes a first-time reader wonder if I am allowed to ignore the
> sample and write my own from scratch, or if the only thing that is
> allowed is to enable what is shipped with .sample suffix.
>
> I wonder it would become less confusing if we placed even _less_
> stress on these sample hooks; I find that saying we ship sample
> hooks that are disabled is probably more confusing.
>
> We do not ship any hook (hence nothing is enabled by definition);
> there are a handful of sample files that you can use when adding
> your own hook by either referencing them or taking them as-is, and
> the latter can be done by removing .sample suffix, which is merely a
> special case convenience.

Will fix this confusion.

>
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index a2f59b1..2f3caf7 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
>>  DESCRIPTION
>>  ---
>>
>> -Hooks are little scripts you can place in `$GIT_DIR/hooks`
>> -directory to trigger action at certain points.  When
>> -'git init' is run, a handful of example hooks are copied into the
>> -`hooks` directory of the new repository, but by default they are
>> -all disabled.  To enable a hook, rename it by removing its `.sample`
>> -suffix.
>> -
>> -NOTE: It is also a requirement for a given hook to be executable.
>> -However - in a freshly initialized repository - the `.sample` files are
>> -executable by default.
>> -
>> -This document describes the currently defined hooks.
>> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
>> +trigger action at certain points. Hooks that don't have the executable
>> +bit set are ignored.
>
> The last sentence is POSIXPERM only, though.

So what does this do on non-POSIX systems?:

const char *find_hook(const char *name)
[...]
strbuf_git_path(, "hooks/%s", name);
if (access(path.buf, X_OK) < 0)
return NULL;

Just always return true I guess.

I'm not going to change this bit, I think that if we have special
systems that don't have +x it makes sense to just document how +x
works on those systems rather than the entirety of the rest of the git
documentation adding caveats every time the executable bit concept
comes up.

>> +When a hook is called in a non-bare repository the working directory
>> +is guaranteed to be the root of the working tree, in a bare repository
>> +the working directory will be the path to the repository. I.e. hooks
>> +don't need to worry about the user's current working directory.
>
> This sentence took me two reads until I mentally substituted "the
> working directory" with "its working diretory", to realize that you
> are 

Re: [PATCH 1/3] githooks.txt: Improve the intro section

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Change the documentation so that:
>
>  * We don't talk about "little scripts". Hooks can be as big as you
>want, and don't have to be scripts, just call them "programs".
>
>  * We note what happens with chdir() before a hook is called, nothing
>documented this explicitly, but the current behavior is
>predictable. It helps a lot to know what directory these hooks will
>be executed from.
>
>  * We don't make claims about the example hooks which may not be true
>depending on the configuration of 'init.templateDir'. Clarify that
>we're talking about the default settings of git-init in those cases,
>and move some of this documentation into git-init's documentation
>about the default templates.
>
>  * We briefly note in the intro that hooks can get their arguments in
>various different ways, and that how exactly is described below for
>each hook.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  Documentation/git-init.txt |  6 +-
>  Documentation/githooks.txt | 32 
>  2 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
> index 8174d27..cf37926 100644
> --- a/Documentation/git-init.txt
> +++ b/Documentation/git-init.txt
> @@ -130,7 +130,11 @@ The template directory will be one of the following (in 
> order):
>   - the default template directory: `/usr/share/git-core/templates`.
>  
>  The default template directory includes some directory structure, suggested
> -"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see 
> linkgit:githooks[5]).
> +"exclude patterns" (see linkgit:gitignore[5]), and example hook files.
> +
> +The example are all disabled by default. To enable a hook, rename it

"sample hooks are all disabled" would be better; if for some unknown
reason you are trying to avoid "sample hooks", "examples are all
disabled" may be acceptable.

> +by removing its `.sample` suffix. See linkgit:githooks[5] for more
> +info on hook execution.

Makes a first-time reader wonder if I am allowed to ignore the
sample and write my own from scratch, or if the only thing that is
allowed is to enable what is shipped with .sample suffix.

I wonder it would become less confusing if we placed even _less_
stress on these sample hooks; I find that saying we ship sample
hooks that are disabled is probably more confusing.

We do not ship any hook (hence nothing is enabled by definition);
there are a handful of sample files that you can use when adding
your own hook by either referencing them or taking them as-is, and
the latter can be done by removing .sample suffix, which is merely a
special case convenience.


> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index a2f59b1..2f3caf7 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
>  DESCRIPTION
>  ---
>  
> -Hooks are little scripts you can place in `$GIT_DIR/hooks`
> -directory to trigger action at certain points.  When
> -'git init' is run, a handful of example hooks are copied into the
> -`hooks` directory of the new repository, but by default they are
> -all disabled.  To enable a hook, rename it by removing its `.sample`
> -suffix.
> -
> -NOTE: It is also a requirement for a given hook to be executable.
> -However - in a freshly initialized repository - the `.sample` files are
> -executable by default.
> -
> -This document describes the currently defined hooks.
> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
> +trigger action at certain points. Hooks that don't have the executable
> +bit set are ignored.

The last sentence is POSIXPERM only, though.

> +When a hook is called in a non-bare repository the working directory
> +is guaranteed to be the root of the working tree, in a bare repository
> +the working directory will be the path to the repository. I.e. hooks
> +don't need to worry about the user's current working directory.

This sentence took me two reads until I mentally substituted "the
working directory" with "its working diretory", to realize that you
are talking about the cwd of the process that runs the hook.

While "is guaranteed" may be technically correct and we have no
intention to change it, it sounds unnecessarily strong.

When a hook is invoked, it is run at the root of the working
tree in a non-bare repository, or in the $GIT_DIR in a bare
repository.

perhaps?

> +Hooks can get their arguments via the environment, command-line
> +arguments, and stdin. See the documentation for each below hook for
> +details.

"each below hook" sounds somewhat ungrammatical.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] githooks.txt: Improve the intro section

[Eric Sunshine]

On Sun, Apr 24, 2016 at 4:20 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Change the documentation so that:
> [...]
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
> @@ -130,7 +130,11 @@ The template directory will be one of the following (in 
> order):
>  The default template directory includes some directory structure, suggested
> -"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see 
> linkgit:githooks[5]).
> +"exclude patterns" (see linkgit:gitignore[5]), and example hook files.
> +
> +The example are all disabled by default. To enable a hook, rename it

s/example/examples/

> +by removing its `.sample` suffix. See linkgit:githooks[5] for more
> +info on hook execution.
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
> +trigger action at certain points. Hooks that don't have the executable
> +bit set are ignored.
> +
> +When a hook is called in a non-bare repository the working directory
> +is guaranteed to be the root of the working tree, in a bare repository
> +the working directory will be the path to the repository. I.e. hooks
> +don't need to worry about the user's current working directory.
> +
> +Hooks can get their arguments via the environment, command-line
> +arguments, and stdin. See the documentation for each below hook for
> +details.
> +
> +When 'git init' is run it may depending on its configuration copy

s/may/&,/
s/configuration/&,/

> +hooks to the new repository, see the the "TEMPLATE DIRECTORY" section
> +in linkgit:git-init[1] for details. When the rest of this document
> +refers to "default hooks" we're talking about the default template
> +shipped with Git.
> +
> +The currently supported hooks are described below.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] githooks.txt: Improve the intro section

[Ævar Arnfjörð Bjarmason]

Change the documentation so that:

 * We don't talk about "little scripts". Hooks can be as big as you
   want, and don't have to be scripts, just call them "programs".

 * We note what happens with chdir() before a hook is called, nothing
   documented this explicitly, but the current behavior is
   predictable. It helps a lot to know what directory these hooks will
   be executed from.

 * We don't make claims about the example hooks which may not be true
   depending on the configuration of 'init.templateDir'. Clarify that
   we're talking about the default settings of git-init in those cases,
   and move some of this documentation into git-init's documentation
   about the default templates.

 * We briefly note in the intro that hooks can get their arguments in
   various different ways, and that how exactly is described below for
   each hook.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-init.txt |  6 +-
 Documentation/githooks.txt | 32 
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 8174d27..cf37926 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -130,7 +130,11 @@ The template directory will be one of the following (in 
order):
  - the default template directory: `/usr/share/git-core/templates`.
 
 The default template directory includes some directory structure, suggested
-"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see 
linkgit:githooks[5]).
+"exclude patterns" (see linkgit:gitignore[5]), and example hook files.
+
+The example are all disabled by default. To enable a hook, rename it
+by removing its `.sample` suffix. See linkgit:githooks[5] for more
+info on hook execution.
 
 EXAMPLES
 
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..2f3caf7 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 ---
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
-directory to trigger action at certain points.  When
-'git init' is run, a handful of example hooks are copied into the
-`hooks` directory of the new repository, but by default they are
-all disabled.  To enable a hook, rename it by removing its `.sample`
-suffix.
-
-NOTE: It is also a requirement for a given hook to be executable.
-However - in a freshly initialized repository - the `.sample` files are
-executable by default.
-
-This document describes the currently defined hooks.
+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
+trigger action at certain points. Hooks that don't have the executable
+bit set are ignored.
+
+When a hook is called in a non-bare repository the working directory
+is guaranteed to be the root of the working tree, in a bare repository
+the working directory will be the path to the repository. I.e. hooks
+don't need to worry about the user's current working directory.
+
+Hooks can get their arguments via the environment, command-line
+arguments, and stdin. See the documentation for each below hook for
+details.
+
+When 'git init' is run it may depending on its configuration copy
+hooks to the new repository, see the the "TEMPLATE DIRECTORY" section
+in linkgit:git-init[1] for details. When the rest of this document
+refers to "default hooks" we're talking about the default template
+shipped with Git.
+
+The currently supported hooks are described below.
 
 HOOKS
 -
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html