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

From: Josh Steadmon <[email-masked]>
Date: Fri, 2 Aug 2019 15:18:55 -0700

On 2019.08.02 11:56, Martin Ågren wrote:
> [Dropped cc-list the first time around. Apologies to those who receive
> this twice...]
> 
> On Fri, 2 Aug 2019 at 00:20, Josh Steadmon <[email-masked]> 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

These all look good to me, thank you for the suggestions! I'll include
them in V4.