On Mon, Apr 25, 2016 at 8:23 PM, Junio C Hamano <[email protected]> wrote:
> Ævar Arnfjörð Bjarmason <[email protected]> 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 <[email protected]>
>> ---
>> 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(&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 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?
Fixed.
>> +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.
Yeah. Now "each hook below".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
