On Fri, May 17, 2024 at 8:31 AM Jelte Fennema-Nio <postg...@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