mirror of
https://git.in.rschanz.org/ryan77627/guix.git
synced 2024-11-07 07:26:13 -05:00
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>
This commit is contained in:
parent
fd20e0d5f4
commit
889a6204f8
2 changed files with 114 additions and 4 deletions
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue