Hi! Just to be clear, I started this thread as discussion on the kind of interaction we reviewers should offer to submitters. I am not suggesting changes in our “quality standards”, nor am I suggesting that one group of people do more work.
Maxime Devos <maximede...@telenet.be> skribis: > Ludovic Courtès schreef op ma 27-06-2022 om 12:10 [+0200]: >> Regarding the review process, I think we should strive for a predictable >> process—not requesting work from the submitter beyond what they can >> expect. Submitters can be expected to follow the written rules¹ and >> perhaps a few more rules (for example, I don’t think we’ve documented >> the fact that #:tests? #f is a last resort and should come with a >> comment). >> >> However, following that principle, we reviewers cannot >> reasonably ask for work beyond that. [...] > > We can ask to do send the notice upstream, if it's in the rules (I > consider this to be part of the unwritten rules). Yes, that’s a reasonable thing to ask for. However, we can only ask for it if the submitter identified a problem themselves; if I’m the one finding a problem, it’s inappropriate to ask someone else to report it on my behalf. > And I don't often have time for addressing the noticed issues and I > have other things to do as well -- I usually limit myself to just > reviewing. I do not intend to take up work beyond that. Of course, and I don’t mean reviewers should do more work! I think the few people that dedicate time to patch review already have more than enough on their plate; the last thing I’d want is to put more pressure on them. We have to care for one another, and that starts by making sure none of us feels a pressure to always do more. > I also consider them to not be rules as in ‘if they are followed, it > WILL be accepted’ but more guidelines like ‘these things are important, > they usually need to be followed, but it's not exhaustive, at times it > might be discovered the list is incomplete’. Agreed. > I don't think that patch submitters can reasonably expect reviewers to > do all the work. Agreed. > Also, previously in discussions about the review process, weren't there > points about a reviewer not having to do everything all at once, they > could choose to review parts they know how to review and have time for > and leave the rest for others? I don’t remember discussions along these lines. I think it can be helpful sometimes, and tricky other times. For example, for a package, I find it hard to split review work. But for a patch series that touches many different things (documentation, build system, importer, whatever), splitting review work among different people may work better. >> Related to that, I think it’s important to have a consistent review >> process. In other words, we should be equally demanding for all >> patches to avoid bad surprises or a feeling of double standard. > > I think I've been consistent in asking to inform upstream of the issues > (*), with no distinction of whether it's a new submitter or an > established one or whatever. I think our standards should be the same whether the submitter is a newcomer or not. The difference is in how we reviewers reply. To a newcomer, reviewers should IMO do the “last kilometer” themselves on behalf of submitters: tweaking the commit log, synopsis/description, indentation, that kind of thing. It’s important because that gives submitters a good experience, it helps them learn, and it’s also low-friction for the reviewer. (That’s also the whole point of guix-mentors.) Naturally, one can be more demanding of seasoned contributors, and I think it’s OK to leave it up to them to fix such things. Thanks, Ludo’. PS: Sorry for the wall of text!
