On Fri, May 17, 2024 at 8:31 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> Such a proposal would basically mean that no-one that cares about
> their patches getting reviews can go on holiday and leave work behind
> during the week before a commit fest. That seems quite undesirable to
> me.
Well, then we make it ten days instead of seven, or give a few days
grace after the CF starts to play catchup, or allow the CfM to make
exceptions.
To be fair, I'm not sure that forcing people to do something like this
is going to solve our problem. I'm very open to other ideas. But one
idea that I'm not open to is to just keep doing what we're doing. It
clearly and obviously does not work.
I just tried scrolling through the CommitFest to a more or less random
spot by flicking the mouse up and down, and then clicked on whatever
ended up in the middle of my screen. I did this four times. Two of
those landed on patches that had extremely long discussion threads
already. One hit a patch from a non-committer that hasn't been
reviewed and needs to be. And the fourth hit a patch from a committer
which maybe could benefit from review but I can already guess that the
patch works fine and unless somebody can find some architectural
downside to the approach taken, there's not really a whole lot to talk
about.
I don't entirely know how to think about that result, but it seems
pretty clear that the unreviewed non-committer patch ought to get
priority, especially if we're talking about the possibility of
non-committers or even junior committers doing drive-by reviews. The
high-quality committer patch might be worth a comment from me, pro or
con or whatever, but it's probably not a great use of time for a more
casual contributor: they probably aren't going to find too much wrong
with it. And the threads with extremely long threads already, well, I
don't know if there's something useful that can be done with those
threads or not, but those patches certainly haven't been ignored.
I'm not sure that any of these should be evicted from the CommitFest,
but we need to think about how to impose some structure on the chaos.
Just classifying all four of those entries as either "Needs Review" or
"Waiting on Author" is pretty useless; then they all look the same,
and they're not. And please don't suggest adding a bunch more status
values that the CfM has to manually police as the solution. We need to
find some way to create a system that does the right thing more often
by default.
--
Robert Haas
EDB: http://www.enterprisedb.com