GNU bug report logs - #66436
[PATCH] doc: Add some guidelines for reviewing.

Previous Next

Full log

Message #8 received at 66436 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 66436 <at> debbugs.gnu.org
Subject: Re: [bug#66436] [PATCH] doc: Add some guidelines for reviewing.
Date: Tue, 10 Oct 2023 15:29:37 +0200

Hi,

This looks great to me!

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> +@cindex LGTM, Looks Good To Me
> +@cindex reviewing, guidelines
> +Review comments should be unambiguous; be as clear and explicit as you
> +can about what you think should be changed, ensuring the author can take
> +action on it.

I’d add a few guidelines here, and perhaps we can make it a bullet
list:

  1. Be clear and explicit about changes you are suggesting, ensuring
     the author can take action on it.  In particular, it is a good idea
     to explicitly ask for new revisions when you want it.

  2. Remain focused: do not change the scope of the work being reviewed.
     For example, if the contribution touches a code that follows a
     pattern deemed unwieldy, it would be unfair to ask the submitter to
     fix all occurrences of that pattern in the code; to put it simply,
     if an problem unrelated to the patch at hand was already there, do
     not ask the submitter to fix it.

  3. Ensure progress.  As they respond to review, submitters may submit
     new revisions of their changes; avoid requesting changes that you
     did not request in the previous round of comments.  Overall, the
     submitter should get a clear sense of progress; the number of items
     open for discussion should clearly decrease over time.

  4. Review is a discussion.  The submitter's and reviewer's views on
     how to achieve a particular change may not always be aligned.  As a
     reviewer, try hard to explain the rationale for suggestions you
     make, and to understand and take into account the submitter's
     motivation for doing things in a certain way.

  5. Aim for finalization.  Reviewing code is time-consuming.  Your goal
     as a reviewer is to put the process on a clear path towards
     integration, possibly with agreed-upon changes, or rejection, with
     a clear and mutually-understood reasoning.  Avoid leaving the
     review process in a lingering state with no clear way out.

I just made it up but I’m sure there are good review guidelines out
there that we could use as an example.

My 2¢,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.