On Fri, May 17, 2024 at 7:11 AM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > Yeah, I 100% agree with this. If a patch bitrots and no one cares enough > to rebase it once in a while, then sure - it's probably fine to mark it > RwF. But forcing all contributors to do a dance every 2 months just to > have a chance someone might take a look, seems ... not great.
I don't think that clicking on a link that someone sends you that asks "hey, is this ready to be reviewed' qualifies as a dance. I'm open to other proposals. But I think that if the proposal is essentially "Hey, let's have the CFMs try harder to do the thing that we've already been asking them to try to do for the last N years," then we might as well just not bother. It's obviously not working, and it's not going to start working because we repeat the same things about bouncing patches more aggressively. I just spent 2 days on it and moved a handful of entries forward. To make a single pass over the whole CommitFest at the rate I was going would take at least two weeks, maybe three. And I am a highly experienced committer and CommitFest manager with good facility in English and a lot of experience arguing on this mailing list. I'm in the best possible position to be able to do this well and efficiently and I can't. So where are we going to find the people who can? I think Andrey Borodin's nearby suggestion of having a separate CfM for each section of the CommitFest does a good job revealing just how bad the current situation is. I agree with him: that would actually work. Asking somebody, for a one-month period, to be responsible for shepherding one-tenth or one-twentieth of the entries in the CommitFest would be a reasonable amount of work for somebody. But we will not find 10 or 20 highly motivated, well-qualified volunteers every other month to do that work; it's a struggle to find one or two highly motivated, well-qualified CommitFest managers, let alone ten or twenty. So I think the right interpretation of his comment is that managing the CommitFest has become about an order of magnitude more difficult than what it needs to be for the task to be done well. > It does seem to me a part of the solution needs to be helping to get > those patches reviewed. I don't know how to do that, but perhaps there's > a way to encourage people to review more stuff, or review stuff from a > wider range of contributors. Say by treating reviews more like proper > contributions. Well, I know that what would encourage *me* to do that is if I could find the patches that fall into this category easily. I'm still not going to spend all of my time on it, but when I do have time to spend on it, I'd rather spend it on stuff that matters than on trying to drain the CommitFest swamp. And right now every time I think "oh, I should spend some time reviewing other people's patches," that time promptly evaporates trying to find the patches that actually need attention. I rarely get beyond the "Bug fixes" section of the CommitFest application before I've used up all my available time, not least because some people have figured out that labelling something they don't like as a Bug fix gets it closer to the top of the CF list, which is alphabetical by section. > Long time ago there was a "rule" that people submitting patches are > expected to do reviews. Perhaps we should be more strict this. This sounds like it's just generating more manual work to add to a system that's already suffering from needing too much manual work. Who would keep track of how many reviews each person is doing, and how many patches they're submitting, and whether those reviews were actually any good, and what would they do about it? One patch that comes to mind here is Thomas Munro's patch for "unaccent: understand ancient Greek "oxia" and other codepoints merged by Unicode". Somebody submitted a bug report and Thomas wrote a patch and added it to the CommitFest. But there are open questions that need to be researched, and this isn't really a priority for Thomas: he was just trying to be nice and put somebody's bug report on a track to resolution. Now, Thomas has many patches in the CommitFest, so if you ask "does he review as much stuff as he has signed up to be reviewed," he clearly doesn't. Let's reject all of his patches, including this one! And if on this specific patch you ask whether the author is staying on top of it, he clearly isn't, so let's reject this one a second time, just for that. Now, what have we accomplished by doing all of that? Not a whole lot, in general, because Thomas is a committer, so he can still commit those patches if he wants, barring objections. However, we have succeeded in kicking them out of our CI system, so if he does commit them, they'll be more likely to break the buildfarm. And in the case of this specific patch, what we've done is punish Thomas for trying to help out somebody who submitted a bug report, and at the same time, made the patch he submitted less visible to anyone who might want to help with it. Wahoo! -- Robert Haas EDB: http://www.enterprisedb.com
