Re: Commitfest Update

On 7/15/22 16:15, Justin Pryzby wrote:
> On Fri, Jul 15, 2022 at 03:17:49PM -0700, Jacob Champion wrote:
>> Also, I would like to see us fold cfbot output into the official CF,
>> rather than do the opposite.
>
> That's been the plan for years :)

Is there something other than lack of round tuits that's blocking
progress? I'm happy to donate more time in this area, but I don't know
if my first patch proposal was helpful (or even on the right list --
pgsql-www, right?).

>>> I'd prefer to see someone
>>> pick one of the patches that hasn't seen a review in 6 (or 16) months, and send
>>> out their most critical review and recommend it be closed, or send an updated
>>> patch with their own fixes as an 0099 patch.
>>
>> That would be cool, but who is "someone"? There have been many, many
>> statements about the amount of CF cruft that needs to be removed. Seems
>> like the CFM is in a decent position to actually do it.
>
> I have hesitated to even try to begin the conversation.
>
> Since a patch author initially creates the CF entry, why shouldn't they also be
> responsible for moving them to the next cf. This serves to indicate a
> continued interest. Ideally they also set back to "needs review" after
> addressing feedback, but I imagine many would forget, and this seems like a
> reasonable task for a CFM to do - look at WOA patches that pass tests to see if
> they're actually WOA.

I agree in principle -- I think, ideally, WoA patches should be
procedurally closed at the end of a commitfest, and carried forward when
the author has actually responded. The problems I can imagine resulting
from this are

- Some reviewers mark WoA _immediately_ upon sending an email. I think
authors should have a small grace period to respond before having their
patches automatically "muted" by the system, if the review happens right
at the end of the CF.

- Carrying forward a closed patch is not actually easy. See below.

> Similarly, patches could be summarily set to "waiting on author" if they didn't
> recently apply, compile, and pass tests. That's the minimum standard.
> However, I think it's better not to do this immediately after the patch stops
> applying/compiling/failing tests, since it's usually easy enough to review it.

It's hard to argue with that, but without automation, this is plenty of
busy work too.

> It should be the author's responsibility to handle that; I don't know why the
> accepted process seems to involve sending dozens of emails to say "needs
> rebase". You're putting to good use of some cfapp email features I don't
> remember seeing used before; that seems much better. Also, it's possible to
> subscribe to CF updates (possibly not a well-advertised, well-known or
> well-used feature).

I don't think it should be reviewers' responsibility to say "needs
rebase" over and over again, and I also don't think a new author should
have to refresh the cfbot every single day to find out whether or not
their patch still applies. These things should be handled by the app.

(Small soapbox, hopefully relevant: I used to be in the camp that making
contributors jump through small procedural hoops would somehow weed out
people who were making low-effort patches. I've since come around to the
position that this just tends to select for people with more free time
and/or persistence. I don't like the idea of raising the busy-work bar
for authors, especially without first fixing the automation problem.)

> I didn't know until recently that when a CF entry is closed, that it's possible
> (I think) for the author themselves to reopen it and "move it to the next CF".
> I suggest to point this out to people; I suppose I'm not the only one who finds
> it offputting when a patch is closed in batch at the end of the month after
> getting only insignificant review.

I think this may have been the goal but I don't think it actually works
in practice. The app refuses to let you carry a RwF patch to a new CF.

--

This is important stuff to discuss, for sure, but I also want to revisit
the thing I put on pause, which is to clear out old Reviewer entries to
make it easier for new reviewers to find work to do. If we're not using
Reviewers as a historical record, is there any reason for me not to keep
clearing that out?

It undoes work that you and others have done to make the historical
record more accurate, and I think that's understandably frustrating. But
I thought we were trying to move away from that usage of it altogether.

--Jacob