[Dropped cc-list the first time around. Apologies to those who receive this twice...]
On Fri, 2 Aug 2019 at 00:20, Josh Steadmon <stead...@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
