summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMaxim Cournoyer <maxim.cournoyer@gmail.com>2023-10-10 08:49:05 -0400
committerMaxim Cournoyer <maxim.cournoyer@gmail.com>2023-11-07 12:33:25 -0500
commit889a6204f871fa47ba1ce12bf4a5e9600576f86a (patch)
treeb622f365d3ae7565418901a9567805ba4a73b3af
parentfd20e0d5f4896c6c8bf440df7c39187df3b3ce2e (diff)
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>
-rw-r--r--doc/contributing.texi111
-rw-r--r--etc/git/gitconfig7
2 files changed, 114 insertions, 4 deletions
diff --git a/doc/contributing.texi b/doc/contributing.texi
index ad8d9d1120c..7a458903be3 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -29,6 +29,7 @@ choice.
29* Submitting Patches:: Share your work. 29* Submitting Patches:: Share your work.
30* Tracking Bugs and Changes:: Keeping it all organized. 30* Tracking Bugs and Changes:: Keeping it all organized.
31* Commit Access:: Pushing to the official repository. 31* Commit Access:: Pushing to the official repository.
32* Reviewing the Work of Others:: Some guidelines for sharing reviews.
32* Updating the Guix Package:: Updating the Guix package definition. 33* Updating the Guix Package:: Updating the Guix package definition.
33* Writing Documentation:: Improving documentation in GNU Guix. 34* Writing Documentation:: Improving documentation in GNU Guix.
34* Translating Guix:: Make Guix speak your native language. 35* Translating Guix:: Make Guix speak your native language.
@@ -1981,7 +1982,12 @@ Debbugs provides a feature called @dfn{usertags} that allows any user to
1981tag any bug with an arbitrary label. Bugs can be searched by usertag, 1982tag any bug with an arbitrary label. Bugs can be searched by usertag,
1982so this is a handy way to organize bugs@footnote{The list of usertags is 1983so this is a handy way to organize bugs@footnote{The list of usertags is
1983public information, and anyone can modify any user's list of usertags, 1984public information, and anyone can modify any user's list of usertags,
1984so keep that in mind if you choose to use this feature.}. 1985so keep that in mind if you choose to use this feature.}. If you use
1986Emacs Debbugs, the entry-point to consult existing usertags is the
1987@samp{C-u M-x debbugs-gnu-usertags} procedure. To set a usertag, press
1988@samp{C} while consulting a bug within the *Guix-Patches* buffer opened
1989with @samp{C-u M-x debbugs-gnu-bugs} buffer, then select @code{usertag}
1990and follow the instructions.
1985 1991
1986For example, to view all the bug reports (or patches, in the case of 1992For example, to view all the bug reports (or patches, in the case of
1987@code{guix-patches}) tagged with the usertag @code{powerpc64le-linux} 1993@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
1994to interact with Debbugs. 2000to interact with Debbugs.
1995 2001
1996In Guix, we are experimenting with usertags to keep track of 2002In Guix, we are experimenting with usertags to keep track of
1997architecture-specific issues. To facilitate collaboration, all our 2003architecture-specific issues, as well as reviewed ones. To facilitate
1998usertags are associated with the single user @code{guix}. The following 2004collaboration, all our usertags are associated with the single user
1999usertags currently exist for that user: 2005@code{guix}. The following usertags currently exist for that user:
2000 2006
2001@table @code 2007@table @code
2002 2008
@@ -2014,6 +2020,9 @@ For issues related to reproducibility. For example, it would be
2014appropriate to assign this usertag to a bug report for a package that 2020appropriate to assign this usertag to a bug report for a package that
2015fails to build reproducibly. 2021fails to build reproducibly.
2016 2022
2023@item reviewed-looks-good
2024You have reviewed the series and it looks good to you (LGTM).
2025
2017@end table 2026@end table
2018 2027
2019If you're a committer and you want to add a usertag, just start using it 2028If 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
2283you're welcome to use your expertise and commit rights to help other 2292you're welcome to use your expertise and commit rights to help other
2284contributors, too! 2293contributors, too!
2285 2294
2295@node Reviewing the Work of Others
2296@section Reviewing the Work of Others
2297
2298Perhaps the biggest action you can do to help GNU Guix grow as a project
2299is to review the work contributed by others. You do not need to be a
2300committer to do so; applying, reading the source, building, linting and
2301running other people's series and sharing your comments about your
2302experience will give some confidence to committers. Basically, you gmust
2303ensure the check list found in the @ref{Submitting Patches} section has
2304been correctly followed. A reviewed patch series should give the best
2305chances for the proposed change to be merged faster, so if a change you
2306would like to see merged hasn't yet been reviewed, this is the most
2307appropriate thing to do!
2308
2309@cindex reviewing, guidelines
2310Review comments should be unambiguous; be as clear and explicit as you
2311can about what you think should be changed, ensuring the author can take
2312action on it. Please try to keep the following guidelines in mind
2313during review:
2314
2315@enumerate
2316@item
2317@emph{Be clear and explicit about changes you are suggesting}, ensuring
2318the author can take action on it. In particular, it is a good idea to
2319explicitly ask for new revisions when you want it.
2320
2321@item
2322@emph{Remain focused: do not change the scope of the work being
2323reviewed.} For example, if the contribution touches code that follows a
2324pattern deemed unwieldy, it would be unfair to ask the submitter to fix
2325all occurrences of that pattern in the code; to put it simply, if a
2326problem unrelated to the patch at hand was already there, do not ask the
2327submitter to fix it.
2328
2329@item
2330@emph{Ensure progress.} As they respond to review, submitters may
2331submit new revisions of their changes; avoid requesting changes that you
2332did not request in the previous round of comments. Overall, the
2333submitter should get a clear sense of progress; the number of items open
2334for discussion should clearly decrease over time.
2335
2336@item
2337@emph{Aim for finalization.} Reviewing code is time-consuming. Your
2338goal as a reviewer is to put the process on a clear path towards
2339integration, possibly with agreed-upon changes, or rejection, with a
2340clear and mutually-understood reasoning. Avoid leaving the review
2341process in a lingering state with no clear way out.
2342
2343@item
2344@emph{Review is a discussion.} The submitter's and reviewer's views on
2345how to achieve a particular change may not always be aligned. To lead
2346the discussion, remain focused, ensure progress and aim for
2347finalization, spending time proportional to the stakes@footnote{The
2348tendency to discuss minute details at length is often referred to as
2349``bikeshedding'', where much time is spent discussing each one's
2350preference for the color of the shed at the expense of progress made on
2351the project to keep bikes dry.}. As a reviewer, try hard to explain the
2352rationale for suggestions you make, and to understand and take into
2353account the submitter's motivation for doing things in a certain way.
2354@end enumerate
2355
2356@cindex LGTM, Looks Good To Me
2357@cindex review tags
2358@cindex Reviewed-by, git trailer
2359When you deem the proposed change adequate and ready for inclusion
2360within Guix, the following well understood/codified
2361@samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>}
2362@footnote{The @samp{Reviewed-by} Git trailer is used by other projects
2363such as Linux, and is understood by third-party tools such as the
2364@samp{b4 am} sub-command, which is able to retrieve the complete
2365submission email thread from a public-inbox instance and add the Git
2366trailers found in replies to the commit patches.} line should be used to
2367sign off as a reviewer, meaning you have reviewed the change and that it
2368looks good to you:
2369
2370@itemize
2371@item
2372If the @emph{whole} series (containing multiple commits) looks good to
2373you, reply with @samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>}
2374to the cover page if it has one, or to the last patch of the series
2375otherwise, adding another @samp{(for the whole series)} comment on the
2376line below to explicit this fact.
2377
2378@item
2379If you instead want to mark a @emph{single commit} as reviewed (but not
2380the whole series), simply reply with
2381@samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>} to that
2382commit message.
2383@end itemize
2384
2385If you are not a committer, you can help others find a @emph{series} you
2386have reviewed more easily by adding a @code{reviewed-looks-good} usertag
2387for the @code{guix} user (@pxref{Debbugs Usertags}).
2388
2286@node Updating the Guix Package 2389@node Updating the Guix Package
2287@section Updating the Guix Package 2390@section Updating the Guix Package
2288 2391
diff --git a/etc/git/gitconfig b/etc/git/gitconfig
index 907ad018045..654a630b180 100644
--- a/etc/git/gitconfig
+++ b/etc/git/gitconfig
@@ -16,3 +16,10 @@
16 to = guix-patches@gnu.org 16 to = guix-patches@gnu.org
17 headerCmd = etc/teams.scm cc-members-header-cmd 17 headerCmd = etc/teams.scm cc-members-header-cmd
18 thread = no 18 thread = no
19
20[b4]
21 attestation-check-dkim = off
22 attestation-policy = off
23 linkmask = https://yhetil.org/guix/%s
24 linktrailermask = https://yhetil.org/guix/%s
25 midmask = https://yhetil.org/guix/%s