Re: [PATCH 3/3] sequencer: run post-commit hook

Johannes Schindelin [off-list ref] writes:

quoted

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.

quoted

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.

quoted

+	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.

quoted

+	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.