[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing.
From: |
Ludovic Courtès |
Subject: |
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing. |
Date: |
Tue, 24 Oct 2023 17:54:11 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> * doc/contributing.texi (Contributing) [Reviewing the Work of Others]: New
> section.
> * doc/contributing.texi (Debbugs Usertags): Expound with Emacs Debbugs
^
Nitpick: no need to repeat the file name.
> +@node Reviewing the Work of Others
> +@section Reviewing the Work of Others
[...]
> +@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:
[...]
> +@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.
Another item came to mind, that could be phrased like this:
Spend time proportional to the stakes. Ensure the discussion focuses
on important aspects of the changes; do not let it be derailed by
objectively unimportant issues@footnote{This situation 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
progress made on the project to keep bikes dry.}. In particular,
issues such as code formatting and other conventions can be dealt with
through automation---e.g., @command{guix lint} and @command{guix
style}---or through documentation (@pxref{Packaging Guidelines}, as an
example).
Maybe that makes the guidelines too long though; your call.
Otherwise LGTM!
Ludo’.
- [bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., (continued)
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., Clément Lassieur, 2023/10/20
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., Simon Tournier, 2023/10/24
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., Simon Tournier, 2023/10/24
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., Maxim Cournoyer, 2023/10/31
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing., Simon Tournier, 2023/10/31
[bug#66436] [PATCH v2] doc: Add some guidelines for reviewing.,
Ludovic Courtès <=
[bug#66436] [PATCH 0/2] Add support for Git Large File Storage (LFS)., Maxim Cournoyer, 2023/10/11