Re: [PATCH 00/31] minimal restart of "config-based-hooks"

On Tue, Jun 01 2021, Derrick Stolee wrote:

On 6/1/2021 2:14 PM, Emily Shaffer wrote:

quoted

On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

After suggesting[1] an another round that the config-based-hook
topic[2] should take a more incremental approach to reach its end goal
I thought I'd try hacking that up.

I think sending this complete reorganization of a long-lived topic
is not helpful, especially because the end-to-end diff is significant.
This series has been extensively tested and incrementally improved
for months, and it would be a waste to start over and lose all of that
hardening.

Felipe already commented downthread on the end-to-end diff aspect of
this. I replied in
http://lore.kernel.org/git/871r9k8zui.fsf@evledraar.gmail.com (local) with the
diff you're probably more interested in looking at.

It's also a but rushed that this comes only a day after the previous
message recommending a reorganization. It would be best to at least
give the original author an opportunity to comment on your idea before
working on this.

I replied toth is as "When you get feedback in the form of restructured
patches[...]" in  https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/ (local)

As noted there I really don't see how sending "here's patches that
implement my suggestion" as opposed to "maybe xyz would be better" is
worse for anyone.

quoted

quoted

So this is a proposed restart of that topic which if the consensus
favors it should replace it, and the config-based hooks topic should
be rebased on top of this.

I'm not entirely sure what you're trying to achieve by sending this
series. It was my impression that the existing config-based-hooks topic
was close to being ready to submit anyway (since Junio mentioned
submitting it a couple revisions ago); rather than churning by reviewing
a different 31-patch topic, and then re-rolling and re-reviewing a
(reduced) config hook topic, wouldn't it be easier on everyone's time to
do a final incremental review on the existing topic and then start in on
bugfixes/feature patches afterwards?

I completely agree here.

What do you think of the issues I raised in [1] ?

Emily's series isn't a pure internal refactoring of existing
functionality, but also an opinionated introduction of new user-facing
functionality.

Now, you may be 100% in agreement with the "opinionated" or
"user-facing" part of that, I'm personally somewhere around 80%.

But to me that clearly makes it a case where we need to as much as
possible get it right *before* it lands on master, because we can't just
incrementally tweak it after it lands without backwards compatibility
concerns. Soon after landing it'll be in a release, have users in the
wild etc.

By default that turns any suggestion of tweaking existing behavior from
a "oh that makes sense, let's fix it" into "well, that's unfortunate,
but it's existing/documented functionality used in the wild".

[2] goes on to mention a couple of other such issues (e.g. the --env
switch to "git hook run"), and this time around I've just been narrowly
focusing on any such issues in the not-config-base-hooks part of this
topic. I.e. the "no user-facing behavior changes yet" part I carved out
into a separate topic.

I'm not telling you which issues those are because I'd like you to tell
me, and then we'll compare notes afterwards. I bet <insert round of
beers at git dev summit or equivalent here> that they're not the same
list.

Anyway, I don't think you will, and I'm not entirely serious. But it's a
real rhetorical point about us being unlikely to come up with the same
result, and thus about the outstanding v9 series being too large to be
readily understood.

quoted

It would have been nice to see a more clear discussion of patch
organization sometime much sooner in the past year and a half since the
project was proposed[3], like maybe in the few iterations of the design
doc which included a rollout plan in July of last year[4]. To me, it
seems late to be overhauling the direction like this, especially after I
asked for opinions and approval on the direction before I started work
in earnest.

I've also seen messages as early as January where Ævar mentioned
wanting to review the series, but not finding the time to do so.
It is reasonable to expect that contributors attempt such major
reorganizations according to reviewers feedback, as long as the
reviewers are timely about delivering that feedback.

I made a case in [2] for it not being too late.

For what it's worth (and some of which is noted in [2]) that's partially
because I for one have found it really hard to keep track of this
series.

Multiple re-rolls (including v8->v9) have some outstanding
discussion/feedback, some of which is addressed in a re-roll, but other
parts (as noted in my [2]) which aren't and are silently omitted.

I'm not blaming Emily for that, I think it's a rather inevitable result
of this thing just being too big to begin with and needing to be split
up.

But that's at least part of the story of feedback at such a late
time. Whenever I've looked at this topic I've spent quite a lot of time
doing that re-reading of past discussions / comparing with the newest
cover letter and noting any omissions etc. myself before even getting to
the point of reading the first patch, and I've sometimes just given up.

1. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/ (local) 
2. https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/ (local)