Re: [PATCH v2 01/30] hook: add 'run' subcommand

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jun 18 2021, Emily Shaffer wrote:

> On Tue, Jun 15, 2021 at 11:36:26AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> > Overall, I think I like the direction your reroll is going - I've needed
>> > some time to process it. Hopefully I'll be able to get through all or
>> > most of the series this week, but there's a lot going on here, too. I'll
>> > do what I can. Thanks for the help.
>> 
>> Yeah, will reply to any qusetions etc; and as noted above my initial
>> goal here was "hey, what about this approach", so if you wanted to pick
>> this up & run with it...
>> 
>> This particular version of the series is at github.com/avar/git.git's
>> es-avar/config-based-hooks-3 b.t.w.
>
> Have finished scanning through the rest of the series, and I think I
> understand your goal a little better - you are not saying "let me take
> over and drive this part of the feature set", which is what I thought
> initially. Instead, you seem to be saying "let's chop it up this way
> instead".

Yes, 30-some patches that both refactor and introduce new behavior are
harder to reason about.

I've also had suggestions about the end-state, but I think whatever we
arrive at doing the scaffolding first without behavior changes makes
sense.

> I don't dislike the reorganization, but I do still wonder whether it's
> a setback to the progress the original series had made. I guess it is
> hard to know - I had thought the original series was pretty much ready
> to go in, therefore making "what if we ordered it this way" moot. But it
> seems that you disagree.

I'm still not sure if I disagree, well, I'm 95% sure I disagree with
some of the end-state, but you never replied to my questions about that:
https://lore.kernel.org/git/87mtv8fww3.fsf@evledaraar.gmail.com/ &
https://lore.kernel.org/git/87lf80l1m6.fsf@evledraar.gmail.com/; So I
don't know for sure, maybe there's things I missed there.

I think since Junio picked up the "base" version of this and it looks
like we're going that way first that's not something we need to discuss
now if you'd like to punt it, but I'd really like to get that cleared up
post-base topic.

In brief summary:

I'm 100% with you on hooks being driven by config, that they aren't is
in hindsight a historical wart. Ditto the parallel execution etc. (which
I'd suggested in an earlier iteration). That's all great.

Where you lose me is the need for having "git hook" be an administrative
interface for it, particularly (as noted in the linked E-Mail) since the
need for that over simply using "git config", or a trivial "git config"
wrapper seems to be fallout from other arbitrary design choices.

I.e. that all the config for a hook needing to be discovered by a
two-pass iteration over the config space (or keeping state), as opposed
to a "hookcfg.<name>.*" (or whatever) prefix.

Maybe that makes sense in the eventual end-state, your series has the
equivalent of "WIP, more will be added later" around that "git hook"
command; but not having the full overview of that I think we can make
simpler inroads into getting us all of the practical featureset we want,
without regretting our choices in command & config interfaces later.

> Anyway, I do hear also that you don't have interest in driving this
> subset to completion, and that's fine. Correct me if I'm wrong.

I submitted a v3 of this (which I forgot to label as such in the
subject) at
https://lore.kernel.org/git/cover-00.27-0000000000-20210617T101216Z-avarab@gmail.com/;
given the timing our E-Mails may have crossed.

But no, I will drive this subset to completion. What I meant with the
"run with it" comment and the earlier reply on v1 of my "base" version
here: https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/

... is that I'd be happier if I managed to just convince you that the
more piecemeal approach is better, and something you'd want to pick up &
drive going forward.

I.e. it's still >95% your code, just re-arranged and split into subsets
of your patches. I really did not mean to "steal" it, it's just
something I hacked up one day to see if the more incremental approach
I'd been suggesting (and felt you were either ignoring or were too busy
to address) was something that could work.