Re: [PATCH v3 0/4] pre-merge-commit hook

["Martin Ågren" <martin.agren@gmail.com>]

[Dropped cc-list the first time around. Apologies to those who receive
this twice...]

On Fri, 2 Aug 2019 at 00:20, Josh Steadmon <steadmon@google.com> wrote:
>
> This series adds a new pre-merge-commit hook, similar in usage to
> pre-commit. It also improves hook testing in t7503, by verifying that
> the correct hooks are run or bypassed as expected.

I really like those test improvements. Now it should be harder to mess
up a future refactoring and run the wrong hook. These hooks are "very
related" so I think this is important.

I've messed with the test a bit and offer these potential improvements
for your consideration. I was lazy and just built this on top of your
series -- if you agree to some or all of these, you'll probably need to
squash them into a few individual patches.

The first four are perhaps more or less a matter of opinion, although I
do think that patch 2/5 is based on an opinion shared by others. ;-)
Patch 5/5 or something like it seems pretty important to me to make
sure that of these two "similar"/"related" hooks with some
"backwards-compatibility-and-code-copying-history" around them, we'd
better pick the right one when they're both available.
 
Feel free to pick and squash as you see fit. (I don't think it makes
sense to have these go in as-are. They really are meant for squashing.)

Martin

Martin Ågren (5):
  t7503: use "&&" in "test_when_finished" rather than ";"
  t7503: avoid touch when mtime doesn't matter
  t7503: simplify file-juggling
  t7503: don't create "actual_hooks" for later appending
  t7503: test failing merge with both hooks available

 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 84 +++++++++++--------
 1 file changed, 50 insertions(+), 34 deletions(-)

-- 
2.23.0.rc0.30.g51cf315870