01/07: doc: Add some guidelines for reviewing.

[guix-commits]

apteryx pushed a commit to branch master
in repository guix.

commit 889a6204f871fa47ba1ce12bf4a5e9600576f86a
Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
AuthorDate: Tue Oct 10 08:49:05 2023 -0400

    doc: Add some guidelines for reviewing.
    
    * doc/contributing.texi (Contributing) [Reviewing the Work of Others]: New
    section.
    (Debbugs Usertags): Expound with Emacs Debbugs information and document the
    'reviewed-looks-good' usertag.
    * etc/git/gitconfig [b4]: New section.
    
    Change-Id: I56630b15ec4fbc5c67e5420dbf2838556a005d6b
    Reviewed-by: Ludovic Courtès <ludo@gnu.org>
---
 doc/contributing.texi | 111 ++++++++++++++++++++++++++++++++++++++++++++++++--
 etc/git/gitconfig     |   7 ++++
 2 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/doc/contributing.texi b/doc/contributing.texi
index ad8d9d1120..7a458903be 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -29,6 +29,7 @@ choice.
 * Submitting Patches::          Share your work.
 * Tracking Bugs and Changes::   Keeping it all organized.
 * Commit Access::               Pushing to the official repository.
+* Reviewing the Work of Others::  Some guidelines for sharing reviews.
 * Updating the Guix Package::   Updating the Guix package definition.
 * Writing Documentation::       Improving documentation in GNU Guix.
 * Translating Guix::            Make Guix speak your native language.
@@ -1981,7 +1982,12 @@ Debbugs provides a feature called @dfn{usertags} that 
allows any user to
 tag any bug with an arbitrary label.  Bugs can be searched by usertag,
 so this is a handy way to organize bugs@footnote{The list of usertags is
 public information, and anyone can modify any user's list of usertags,
-so keep that in mind if you choose to use this feature.}.
+so keep that in mind if you choose to use this feature.}.  If you use
+Emacs Debbugs, the entry-point to consult existing usertags is the
+@samp{C-u M-x debbugs-gnu-usertags} procedure.  To set a usertag, press
+@samp{C} while consulting a bug within the *Guix-Patches* buffer opened
+with @samp{C-u M-x debbugs-gnu-bugs} buffer, then select @code{usertag}
+and follow the instructions.
 
 For example, to view all the bug reports (or patches, in the case of
 @code{guix-patches}) tagged with the usertag @code{powerpc64le-linux}
@@ -1994,9 +2000,9 @@ documentation for Debbugs or the documentation for 
whatever tool you use
 to interact with Debbugs.
 
 In Guix, we are experimenting with usertags to keep track of
-architecture-specific issues.  To facilitate collaboration, all our
-usertags are associated with the single user @code{guix}.  The following
-usertags currently exist for that user:
+architecture-specific issues, as well as reviewed ones.  To facilitate
+collaboration, all our usertags are associated with the single user
+@code{guix}.  The following usertags currently exist for that user:
 
 @table @code
 
@@ -2014,6 +2020,9 @@ For issues related to reproducibility.  For example, it 
would be
 appropriate to assign this usertag to a bug report for a package that
 fails to build reproducibly.
 
+@item reviewed-looks-good
+You have reviewed the series and it looks good to you (LGTM).
+
 @end table
 
 If you're a committer and you want to add a usertag, just start using it
@@ -2283,6 +2292,100 @@ only push their own awesome changes, but also offer 
some of their time
 you're welcome to use your expertise and commit rights to help other
 contributors, too!
 
+@node Reviewing the Work of Others
+@section Reviewing the Work of Others
+
+Perhaps the biggest action you can do to help GNU Guix grow as a project
+is to review the work contributed by others.  You do not need to be a
+committer to do so; applying, reading the source, building, linting and
+running other people's series and sharing your comments about your
+experience will give some confidence to committers.  Basically, you gmust
+ensure the check list found in the @ref{Submitting Patches} section has
+been correctly followed.  A reviewed patch series should give the best
+chances for the proposed change to be merged faster, so if a change you
+would like to see merged hasn't yet been reviewed, this is the most
+appropriate thing to do!
+
+@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.  Please try to keep the following guidelines in mind
+during review:
+
+@enumerate
+@item
+@emph{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.
+
+@item
+@emph{Remain focused: do not change the scope of the work being
+reviewed.}  For example, if the contribution touches 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 a
+problem unrelated to the patch at hand was already there, do not ask the
+submitter to fix it.
+
+@item
+@emph{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.
+
+@item
+@emph{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.
+
+@item
+@emph{Review is a discussion.}  The submitter's and reviewer's views on
+how to achieve a particular change may not always be aligned.  To lead
+the discussion, remain focused, ensure progress and aim for
+finalization, spending time proportional to the stakes@footnote{The
+tendency to discuss minute details at length is often referred to as
+``bikeshedding'', where much time is spent discussing each one's
+preference for the color of the shed at the expense of progress made on
+the project to keep bikes dry.}.  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.
+@end enumerate
+
+@cindex LGTM, Looks Good To Me
+@cindex review tags
+@cindex Reviewed-by, git trailer
+When you deem the proposed change adequate and ready for inclusion
+within Guix, the following well understood/codified
+@samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>}
+@footnote{The @samp{Reviewed-by} Git trailer is used by other projects
+such as Linux, and is understood by third-party tools such as the
+@samp{b4 am} sub-command, which is able to retrieve the complete
+submission email thread from a public-inbox instance and add the Git
+trailers found in replies to the commit patches.} line should be used to
+sign off as a reviewer, meaning you have reviewed the change and that it
+looks good to you:
+
+@itemize
+@item
+If the @emph{whole} series (containing multiple commits) looks good to
+you, reply with 
@samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>}
+to the cover page if it has one, or to the last patch of the series
+otherwise, adding another @samp{(for the whole series)} comment on the
+line below to explicit this fact.
+
+@item
+If you instead want to mark a @emph{single commit} as reviewed (but not
+the whole series), simply reply with
+@samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>} to that
+commit message.
+@end itemize
+
+If you are not a committer, you can help others find a @emph{series} you
+have reviewed more easily by adding a @code{reviewed-looks-good} usertag
+for the @code{guix} user (@pxref{Debbugs Usertags}).
+
 @node Updating the Guix Package
 @section Updating the Guix Package
 
diff --git a/etc/git/gitconfig b/etc/git/gitconfig
index 907ad01804..654a630b18 100644
--- a/etc/git/gitconfig
+++ b/etc/git/gitconfig
@@ -16,3 +16,10 @@
         to = guix-patches@gnu.org
         headerCmd = etc/teams.scm cc-members-header-cmd
         thread = no
+
+[b4]
+        attestation-check-dkim = off
+        attestation-policy = off
+        linkmask = https://yhetil.org/guix/%s
+        linktrailermask = https://yhetil.org/guix/%s
+        midmask = https://yhetil.org/guix/%s