diff options
| author | Maxim Cournoyer <maxim.cournoyer@gmail.com> | 2023-10-10 08:49:05 -0400 |
|---|---|---|
| committer | Maxim Cournoyer <maxim.cournoyer@gmail.com> | 2023-11-07 12:33:25 -0500 |
| commit | 889a6204f871fa47ba1ce12bf4a5e9600576f86a (patch) | |
| tree | b622f365d3ae7565418901a9567805ba4a73b3af | |
| parent | fd20e0d5f4896c6c8bf440df7c39187df3b3ce2e (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.texi | 111 | ||||
| -rw-r--r-- | etc/git/gitconfig | 7 |
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 | |||
| 1981 | tag any bug with an arbitrary label. Bugs can be searched by usertag, | 1982 | tag any bug with an arbitrary label. Bugs can be searched by usertag, |
| 1982 | so this is a handy way to organize bugs@footnote{The list of usertags is | 1983 | so this is a handy way to organize bugs@footnote{The list of usertags is |
| 1983 | public information, and anyone can modify any user's list of usertags, | 1984 | public information, and anyone can modify any user's list of usertags, |
| 1984 | so keep that in mind if you choose to use this feature.}. | 1985 | so keep that in mind if you choose to use this feature.}. If you use |
| 1986 | Emacs 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 | ||
| 1989 | with @samp{C-u M-x debbugs-gnu-bugs} buffer, then select @code{usertag} | ||
| 1990 | and follow the instructions. | ||
| 1985 | 1991 | ||
| 1986 | For example, to view all the bug reports (or patches, in the case of | 1992 | For 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 | |||
| 1994 | to interact with Debbugs. | 2000 | to interact with Debbugs. |
| 1995 | 2001 | ||
| 1996 | In Guix, we are experimenting with usertags to keep track of | 2002 | In Guix, we are experimenting with usertags to keep track of |
| 1997 | architecture-specific issues. To facilitate collaboration, all our | 2003 | architecture-specific issues, as well as reviewed ones. To facilitate |
| 1998 | usertags are associated with the single user @code{guix}. The following | 2004 | collaboration, all our usertags are associated with the single user |
| 1999 | usertags 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 | |||
| 2014 | appropriate to assign this usertag to a bug report for a package that | 2020 | appropriate to assign this usertag to a bug report for a package that |
| 2015 | fails to build reproducibly. | 2021 | fails to build reproducibly. |
| 2016 | 2022 | ||
| 2023 | @item reviewed-looks-good | ||
| 2024 | You have reviewed the series and it looks good to you (LGTM). | ||
| 2025 | |||
| 2017 | @end table | 2026 | @end table |
| 2018 | 2027 | ||
| 2019 | If you're a committer and you want to add a usertag, just start using it | 2028 | 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 | |||
| 2283 | you're welcome to use your expertise and commit rights to help other | 2292 | you're welcome to use your expertise and commit rights to help other |
| 2284 | contributors, too! | 2293 | contributors, too! |
| 2285 | 2294 | ||
| 2295 | @node Reviewing the Work of Others | ||
| 2296 | @section Reviewing the Work of Others | ||
| 2297 | |||
| 2298 | Perhaps the biggest action you can do to help GNU Guix grow as a project | ||
| 2299 | is to review the work contributed by others. You do not need to be a | ||
| 2300 | committer to do so; applying, reading the source, building, linting and | ||
| 2301 | running other people's series and sharing your comments about your | ||
| 2302 | experience will give some confidence to committers. Basically, you gmust | ||
| 2303 | ensure the check list found in the @ref{Submitting Patches} section has | ||
| 2304 | been correctly followed. A reviewed patch series should give the best | ||
| 2305 | chances for the proposed change to be merged faster, so if a change you | ||
| 2306 | would like to see merged hasn't yet been reviewed, this is the most | ||
| 2307 | appropriate thing to do! | ||
| 2308 | |||
| 2309 | @cindex reviewing, guidelines | ||
| 2310 | Review comments should be unambiguous; be as clear and explicit as you | ||
| 2311 | can about what you think should be changed, ensuring the author can take | ||
| 2312 | action on it. Please try to keep the following guidelines in mind | ||
| 2313 | during review: | ||
| 2314 | |||
| 2315 | @enumerate | ||
| 2316 | @item | ||
| 2317 | @emph{Be clear and explicit about changes you are suggesting}, ensuring | ||
| 2318 | the author can take action on it. In particular, it is a good idea to | ||
| 2319 | explicitly ask for new revisions when you want it. | ||
| 2320 | |||
| 2321 | @item | ||
| 2322 | @emph{Remain focused: do not change the scope of the work being | ||
| 2323 | reviewed.} For example, if the contribution touches code that follows a | ||
| 2324 | pattern deemed unwieldy, it would be unfair to ask the submitter to fix | ||
| 2325 | all occurrences of that pattern in the code; to put it simply, if a | ||
| 2326 | problem unrelated to the patch at hand was already there, do not ask the | ||
| 2327 | submitter to fix it. | ||
| 2328 | |||
| 2329 | @item | ||
| 2330 | @emph{Ensure progress.} As they respond to review, submitters may | ||
| 2331 | submit new revisions of their changes; avoid requesting changes that you | ||
| 2332 | did not request in the previous round of comments. Overall, the | ||
| 2333 | submitter should get a clear sense of progress; the number of items open | ||
| 2334 | for discussion should clearly decrease over time. | ||
| 2335 | |||
| 2336 | @item | ||
| 2337 | @emph{Aim for finalization.} Reviewing code is time-consuming. Your | ||
| 2338 | goal as a reviewer is to put the process on a clear path towards | ||
| 2339 | integration, possibly with agreed-upon changes, or rejection, with a | ||
| 2340 | clear and mutually-understood reasoning. Avoid leaving the review | ||
| 2341 | process 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 | ||
| 2345 | how to achieve a particular change may not always be aligned. To lead | ||
| 2346 | the discussion, remain focused, ensure progress and aim for | ||
| 2347 | finalization, spending time proportional to the stakes@footnote{The | ||
| 2348 | tendency to discuss minute details at length is often referred to as | ||
| 2349 | ``bikeshedding'', where much time is spent discussing each one's | ||
| 2350 | preference for the color of the shed at the expense of progress made on | ||
| 2351 | the project to keep bikes dry.}. As a reviewer, try hard to explain the | ||
| 2352 | rationale for suggestions you make, and to understand and take into | ||
| 2353 | account 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 | ||
| 2359 | When you deem the proposed change adequate and ready for inclusion | ||
| 2360 | within 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 | ||
| 2363 | such 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 | ||
| 2365 | submission email thread from a public-inbox instance and add the Git | ||
| 2366 | trailers found in replies to the commit patches.} line should be used to | ||
| 2367 | sign off as a reviewer, meaning you have reviewed the change and that it | ||
| 2368 | looks good to you: | ||
| 2369 | |||
| 2370 | @itemize | ||
| 2371 | @item | ||
| 2372 | If the @emph{whole} series (containing multiple commits) looks good to | ||
| 2373 | you, reply with @samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>} | ||
| 2374 | to the cover page if it has one, or to the last patch of the series | ||
| 2375 | otherwise, adding another @samp{(for the whole series)} comment on the | ||
| 2376 | line below to explicit this fact. | ||
| 2377 | |||
| 2378 | @item | ||
| 2379 | If you instead want to mark a @emph{single commit} as reviewed (but not | ||
| 2380 | the whole series), simply reply with | ||
| 2381 | @samp{Reviewed-by:@tie{}Your@tie{}Name<your-email@@example.com>} to that | ||
| 2382 | commit message. | ||
| 2383 | @end itemize | ||
| 2384 | |||
| 2385 | If you are not a committer, you can help others find a @emph{series} you | ||
| 2386 | have reviewed more easily by adding a @code{reviewed-looks-good} usertag | ||
| 2387 | for 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 | ||
