From: "Ludovic Courtès" <ludo@gnu.org>
To: Maxime Devos <maximedevos@telenet.be>
Cc: Tobias Kortkamp <tobias.kortkamp@gmail.com>, guix-devel@gnu.org
Subject: Re: Dealing with upstream issues
Date: Thu, 30 Jun 2022 13:53:30 +0200 [thread overview]
Message-ID: <87sfnm74g5.fsf@gnu.org> (raw)
In-Reply-To: <6d2b1052b3e63a67c42c4e6ce431b3f1bb4b4605.camel@telenet.be> (Maxime Devos's message of "Mon, 27 Jun 2022 12:30:48 +0200")
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 <maximedevos@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!
next prev parent reply other threads:[~2022-06-30 11:59 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <6d31ff958ec0c75cbba8324a275315d195a54902.1653045472.git.tobias.kortkamp@gmail.com>
[not found] ` <87sfntu6ft.fsf@gnu.org>
[not found] ` <b6d482002f7773e65e02dbbbf354e1c0178b823a.camel@telenet.be>
2022-06-27 10:10 ` Dealing with upstream issues Ludovic Courtès
2022-06-27 10:30 ` Maxime Devos
2022-06-27 10:37 ` Maxime Devos
2022-06-27 12:32 ` zimoun
2022-06-27 14:20 ` Maxime Devos
2022-06-27 15:06 ` zimoun
2022-06-27 15:41 ` Maxime Devos
2022-06-28 11:01 ` zimoun
2022-06-28 12:21 ` Maxime Devos
2022-06-28 16:21 ` zimoun
2022-06-28 16:47 ` Maxime Devos
2022-06-28 17:36 ` zimoun
2022-06-28 20:03 ` Maxime Devos
2022-06-28 12:22 ` Maxime Devos
2022-06-28 12:31 ` Maxime Devos
2022-06-28 16:25 ` zimoun
2022-06-28 16:47 ` Maxime Devos
2022-06-29 6:07 ` bokr
2022-06-29 7:29 ` Missing tags in Debbugs? zimoun
2022-06-29 13:45 ` Bengt Richter
2022-06-30 11:53 ` Ludovic Courtès [this message]
2022-06-27 12:53 ` Dealing with upstream issues zimoun
2022-06-27 14:32 ` Maxime Devos
2022-06-27 15:23 ` zimoun
2022-06-27 15:47 ` Maxime Devos
2022-06-27 16:03 ` Maxime Devos
2022-06-27 16:14 ` Maxime Devos
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sfnm74g5.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=guix-devel@gnu.org \
--cc=maximedevos@telenet.be \
--cc=tobias.kortkamp@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).