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

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

flat view

From: Emily Shaffer <hidden>
Date: 2021-06-22 00:01:00

On Sun, Jun 20, 2021 at 09:30:59PM +0200, Ævar Arnfjörð Bjarmason wrote:
On Fri, Jun 18 2021, Emily Shaffer wrote:
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.
quoted
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/ (local); 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.
quoted
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/ (local);
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/ (local)

... 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.
Ok. Thanks for clarifying.

Yes, I do like this direction, and I'm pleased you were able to chop it
up in a way where partial submission made sense - I struggled with that,
myself. Yes, I am excited that you want to drive this series :) :) and
will be happy to rebase on top of it.

I'll have a look at the range-diff for v3 this week. Thanks.

 - 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