mirror of
https://github.com/git/git.git
synced 2026-06-19 15:39:47 +02:00
Merge branch 'wy/doc-clarify-review-replies' into seen
Documentation on community contribution guidelines has been updated to encourage replying to review comments before rerolling, and to advise a default limit of at most one reroll per day to give reviewers across different time zones enough time to participate. * wy/doc-clarify-review-replies: doc: advise batching patch rerolls doc: encourage review replies before rerolling
This commit is contained in:
@@ -1416,6 +1416,26 @@ previous one" patches over 2 days), reviewers would strongly prefer if a
|
||||
single polished version came 2 days later instead, and that version with
|
||||
fewer mistakes were the only one they would need to review.
|
||||
|
||||
This consideration applies not only when going from the initial patch to v2,
|
||||
but also to later iterations of the same series. There is no fixed rule for how
|
||||
long to wait before sending a new version. A useful default is to send at most
|
||||
one new version of the same patch series per day. This gives multiple reviewers
|
||||
time to comment, gives reviewers across time zones a fair chance to
|
||||
participate, lets you batch feedback together, and gives you time to think
|
||||
through the comments you received. Knowing that you should not immediately send
|
||||
another version also encourages you to review the patches more carefully before
|
||||
sending them, catch small mistakes such as typos and off-by-one errors
|
||||
yourself, and let reviewers spend more of their attention on design,
|
||||
algorithms, and other substantial issues.
|
||||
|
||||
The right timing depends on the topic and the feedback. Larger series usually
|
||||
need more review time. If the only comments so far are minor, such as typo
|
||||
fixes, it often makes sense to wait a little longer in case deeper reviews are
|
||||
still coming. If the comments require substantial rework, sending a new version
|
||||
sooner may save reviewers from spending time on a version you already know will
|
||||
change significantly. If the topic is close to being accepted and the remaining
|
||||
comments are small, a quicker new version may also be fine.
|
||||
|
||||
|
||||
[[reviewing]]
|
||||
=== Responding to Reviews
|
||||
@@ -1423,11 +1443,13 @@ fewer mistakes were the only one they would need to review.
|
||||
After a few days, you will hopefully receive a reply to your patchset with some
|
||||
comments. Woohoo! Now you can get back to work.
|
||||
|
||||
It's good manners to reply to each comment, notifying the reviewer that you have
|
||||
made the change suggested, feel the original is better, or that the comment
|
||||
inspired you to do something a new way which is superior to both the original
|
||||
and the suggested change. This way reviewers don't need to inspect your v2 to
|
||||
figure out whether you implemented their comment or not.
|
||||
It's good manners to reply to each comment in the mailing list discussion
|
||||
instead of letting the next version of your patch be your only response. Tell
|
||||
the reviewer whether you plan to make the suggested change, keep the original,
|
||||
or pursue a different approach. This way reviewers can respond to your reasoning
|
||||
before you spend time preparing a version they may not agree with, and later do
|
||||
not need to inspect your v2 to figure out whether you implemented their comment
|
||||
or not.
|
||||
|
||||
Reviewers may ask you about what you wrote in the patchset, either in
|
||||
the proposed commit log message or in the changes themselves. You
|
||||
|
||||
@@ -48,8 +48,12 @@ area.
|
||||
|
||||
. You get comments and suggestions for improvements. You may even get
|
||||
them in an "on top of your change" patch form. You are expected to
|
||||
respond to them with "Reply-All" on the mailing list, while taking
|
||||
them into account while preparing an updated set of patches.
|
||||
respond to them with "Reply-All" on the mailing list, instead of
|
||||
letting an updated patch series be your only response. Tell
|
||||
reviewers which suggestions you plan to use, which ones you disagree
|
||||
with, and when a comment leads you to consider a different approach.
|
||||
Use these replies and any follow-up discussion as input when
|
||||
preparing an updated set of patches.
|
||||
+
|
||||
Be particularly mindful of critiques regarding the high-level design
|
||||
or viability of your proposal (e.g., questioning if the feature is
|
||||
@@ -69,7 +73,14 @@ considered ready for merging.
|
||||
It is often beneficial to allow some time for reviewers to provide
|
||||
feedback before sending a new version, rather than sending an updated
|
||||
series immediately after receiving a review. This helps collect broader
|
||||
input and avoids unnecessary churn from many rapid iterations.
|
||||
input, gives reviewers in different time zones a fair chance to comment,
|
||||
and avoids unnecessary churn from many rapid iterations. Waiting also
|
||||
encourages you to polish each version before sending it, so reviewers can
|
||||
focus on substantial issues rather than typos or other small mistakes.
|
||||
+
|
||||
As a rough default, avoid sending more than one new version of the same
|
||||
series per day, while considering the size of the series, the depth of
|
||||
review, and how close the topic is to being accepted.
|
||||
|
||||
. These early update iterations are expected to be full replacements,
|
||||
not incremental updates on top of what you posted already. If you
|
||||
@@ -660,7 +671,10 @@ grouped into their own e-mail thread to help readers find all parts of the
|
||||
series. To that end, send them as replies to either an additional "cover
|
||||
letter" message (see below), the first patch, or the respective preceding patch.
|
||||
Here is a link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on
|
||||
how to submit updated versions of a patch series.
|
||||
how to submit updated versions of a patch series. Before sending another
|
||||
version, make sure you have answered meaningful review comments in the existing
|
||||
discussion. Also give reviewers enough time to comment before sending another
|
||||
version.
|
||||
|
||||
If your log message (including your name on the
|
||||
`Signed-off-by:` trailer) is not writable in ASCII, make sure that
|
||||
|
||||
Reference in New Issue
Block a user