Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index d898a57f5d..adb8c89c60 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1653,7 +1653,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>
>> repo_rerere(the_repository, 0);
>> run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
>> - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
>> + run_post_commit_hook(use_editor, get_index_file());
>
> Does it really make sense to abstract the hook name away? It adds a lot
> of churn for just two callers...
After looking at the three patches, I do not think so.
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index d2f1d5bd23..d9217235b6 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -1467,4 +1467,21 @@ test_expect_success 'valid author header when author contains single quote' '
>> test_cmp expected actual
>> '
>>
>> +test_expect_success 'post-commit hook is called' '
>> + test_when_finished "rm -f .git/hooks/post-commit commits" &&
>> + mkdir -p .git/hooks &&
>> + write_script .git/hooks/post-commit <<-\EOS &&
>> + git rev-parse HEAD >>commits
>
> Should `commits` be initialized before this script is written, e.g.
> using
>
> >commits &&
Yes.
>> + git rev-list --no-walk=unsorted HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} \
>> + HEAD@{1} HEAD >expected &&
>
> Wouldn't this be better as:
>
> git rev-parse HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} HEAD@{1} HEAD \
> >expect &&
>
Yes.
>> + test_cmp expected commits
>
> We usually use the name `expect` instead of `expected` in the test
> suite.
And the actual output file is called 'actual'.
Thanks.
