On Thu, May 16, 2024 at 5:46 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Right, so what can we do about that? Does Needs Review state need to > be subdivided, and if so how?
It doesn't really matter how many states we have if they're not set accurately. > At this point it seems like there's consensus to have a "parking" > section of the CF app, separate from the time-boxed CFs, and I hope > somebody will go make that happen. But I don't think that's our only > issue, so we need to keep thinking about what should be improved. I do *emphatically* think we need a parking lot. And a better integration between commitfest.postgresql.org and the cfbot, too. It's just ridiculous that more work hasn't been put into this. I'm not faulting Thomas or anyone else -- I mean, it's not his job to build the infrastructure the project needs any more than it is anyone else's -- but for a project of the size and importance of PostgreSQL to take years to make minor improvements to this kind of critical infrastructure is kind of nuts. If we don't have enough volunteers, let's go recruit some more and promise them cookies. I think you're overestimating the extent to which the problem is "we don't reject enough patches". That *is* a problem, but it seems we have also started rejecting some patches that just never really got much attention for no particularly good reason, while letting other things linger that didn't really get that much more attention, or which were objectively much worse ideas. I think that one place where the process is breaking down is in the tacit assumption that the person who wrote the patch wants to get it committed. In some cases, people park things in the CF for CI runs without a strong intention of pursuing them; in other cases, the patch author is in no kind of rush. I think we need to move to a system where patches exist independent of a CommitFest, and get CI run on them unless they get explicitly closed or are too inactive or some other criterion that boils down to nobody cares any more. Then, we need to get whatever subset of those patches need to be reviewed in a particular CommitFest added to that CommitFest. For example, imagine that the CommitFest is FORCIBLY empty until a week before it starts. You can still register patches in the system generally, but that just means they get CI runs, not that they're scheduled to be reviewed. A week before the CommitFest, everyone who has a patch registered in the system that still applies gets an email saying "click here if you think this patch should be reviewed in the upcoming CommitFest -- if you don't care about the patch any more or it needs more work before other people review it, don't click here". Then, the CommitFest ends up containing only the things where the patch author clicked there during that week. For bonus points, suppose we make it so that when you click the link, it takes you to a box where you can type in a text comment that will display in the app, explaining why your patch needs review: "Robert says the wire protocol changes in this patch are wrong, but I think he's full of hot garbage and want a second opinion!" (a purely hypothetical example, I'm sure) If you put in a comment like this when you register your patch for the CommitFest, it gets a sparkly gold border and floats to the top of the list, or we mail you a Kit-Kat bar, or something. I don't know. The point is - we need a much better signal to noise ratio here. I bet the number of patches in the CommitFest that actually need review is something like 25% of the total. The rest are things that are just parked there by a committer, or that the author doesn't care about right now, or that are already being actively discussed, or where there's not a clear way forward. We could create new statuses for all of those states - "Parked", "In Hibernation," "Under Discussion," and "Unclear" - but I think that's missing the point. What we really want is to not see that stuff in the first place. It's a CommitFest, not once-upon-a-time-I-wrote-a-patch-Fest. -- Robert Haas EDB: http://www.enterprisedb.com
