Thread (581 messages) flat view 581 messages, 16 authors, 2021-10-12

Re: [PATCH v4 8/9] commit: use config-based hooks

From: Emily Shaffer <hidden>
Date: 2020-10-06 19:08:50

On Mon, Oct 05, 2020 at 04:48:39PM -0700, Jonathan Nieder wrote:
Emily Shaffer wrote:
quoted
On Wed, Sep 23, 2020 at 04:47:34PM -0700, Jonathan Tan wrote:
quoted
quoted
 - As for whether (1) it is OK for none of the hooks to be converted (and
   instead rely on the user to edit their hook scripts to call "git hook
   run ???"), or if (2) we should require some hooks to be
   converted, or if (3) we should require all hooks to be converted: I'd
   rather have (2) or (3) so that we don't have dead code. I prefer (3),
   especially since a reviewer wouldn't have to worry about leftover
   usages of old functions like find_hook() (as I mentioned at the start
   of this email), but I'm not fully opposed to (2) either.
I personally prefer (3) - I think the user experience with (2) in a
release (or even in 'next', which all Googlers use) is pretty bad. The
downside, of course, is that a large topic gets merged all at once and
makes some pretty nasty reviewer overhead.
One approach is to build up a series with "git hook run" and "git hook
list" demonstrating and testing the functionality and [PATCH n+1/n]
extra patches at the end converting existing hooks.  The user
experience from "git hook run" and even "git hook list" supporting a
preview of the future without built-in commands living in that future
yet would not be so bad, methinks.  And then a final series could
update the built-in commands' usage of hooks and would still be fairly
small.

In other words, I think I like (1), except *without* the
recommendation for users to edit their hook scripts to call "git hook
run" --- instead, the recommendation would be "try running this
command if you want to see what hooks will do in the future".
Ok. I'll fix up the wording in the design doc and follow through with my
plan to split the series into two parts.

 - Emily
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help