Skip to content

Commit c691f0d

Browse files
author
dscho
committed
Update manual pages (manually forced rebuild)
Updated via the `update-git-version-and-manual-pages.yml` GitHub workflow.
1 parent 1deace4 commit c691f0d

File tree

82 files changed

+62661
-0
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+62661
-0
lines changed

external/docs/asciidoc/0317be45e7d36ce28b9d4ac17272a2d4ca57d620

+754
Large diffs are not rendered by default.

external/docs/asciidoc/13e8ae1a74ea153fbfe36c0a08113b8f89595b00

+589
Large diffs are not rendered by default.

external/docs/asciidoc/201fe4605e6de6698b4916de3fa8b194a71c5f64

+610
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
Reviewing Patches in the Git Project
2+
====================================
3+
4+
Introduction
5+
------------
6+
The Git development community is a widely distributed, diverse, ever-changing
7+
group of individuals. Asynchronous communication via the Git mailing list poses
8+
unique challenges when reviewing or discussing patches. This document contains
9+
some guiding principles and helpful tools you can use to make your reviews both
10+
more efficient for yourself and more effective for other contributors.
11+
12+
Note that none of the recommendations here are binding or in any way a
13+
requirement of participation in the Git community. They are provided as a
14+
resource to supplement your skills as a contributor.
15+
16+
Principles
17+
----------
18+
19+
Selecting patch(es) to review
20+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
21+
If you are looking for a patch series in need of review, start by checking
22+
the latest "What's cooking in git.git" email
23+
(https://lore.kernel.org/git/[email protected]/[example]). The "What's
24+
cooking" emails & replies can be found using the query `s:"What's cooking"` on
25+
the https://lore.kernel.org/git/[`lore.kernel.org` mailing list archive];
26+
alternatively, you can find the contents of the "What's cooking" email tracked
27+
in `whats-cooking.txt` on the `todo` branch of Git. Topics tagged with "Needs
28+
review" and those in the "[New Topics]" section are typically those that would
29+
benefit the most from additional review.
30+
31+
Patches can also be searched manually in the mailing list archive using a query
32+
like `s:"PATCH" -s:"Re:"`. You can browse these results for topics relevant to
33+
your expertise or interest.
34+
35+
If you've already contributed to Git, you may also be CC'd in another
36+
contributor's patch series. These are topics where the author feels that your
37+
attention is warranted. This may be because their patch changes something you
38+
wrote previously (making you a good judge of whether the new approach does or
39+
doesn't work), or because you have the expertise to provide an exceptionally
40+
helpful review. There is no requirement to review these patches but, in the
41+
spirit of open source collaboration, you should strongly consider doing so.
42+
43+
Reviewing patches
44+
~~~~~~~~~~~~~~~~~
45+
While every contributor takes their own approach to reviewing patches, here are
46+
some general pieces of advice to make your reviews as clear and helpful as
47+
possible. The advice is broken into two rough categories: high-level reviewing
48+
guidance, and concrete tips for interacting with patches on the mailing list.
49+
50+
==== High-level guidance
51+
- Remember to review the content of commit messages for correctness and clarity,
52+
in addition to the code change in the patch's diff. The commit message of a
53+
patch should accurately and fully explain the code change being made in the
54+
diff.
55+
56+
- Reviewing test coverage is an important - but easy to overlook - component of
57+
reviews. A patch's changes may be covered by existing tests, or new tests may
58+
be introduced to exercise new behavior. Checking out a patch or series locally
59+
allows you to manually mutate lines of new & existing tests to verify expected
60+
pass/fail behavior. You can use this information to verify proper coverage or
61+
to suggest additional tests the author could add.
62+
63+
- When providing a recommendation, be as clear as possible about whether you
64+
consider it "blocking" (the code would be broken or otherwise made worse if an
65+
issue isn't fixed) or "non-blocking" (the patch could be made better by taking
66+
the recommendation, but acceptance of the series does not require it).
67+
Non-blocking recommendations can be particularly ambiguous when they are
68+
related to - but outside the scope of - a series ("nice-to-have"s), or when
69+
they represent only stylistic differences between the author and reviewer.
70+
71+
- When commenting on an issue, try to include suggestions for how the author
72+
could fix it. This not only helps the author to understand and fix the issue,
73+
it also deepens and improves your understanding of the topic.
74+
75+
- Reviews do not need to exclusively point out problems. Positive
76+
reviews indicate that it is not only the original author of the
77+
patches who care about the issue the patches address, and are
78+
highly encouraged.
79+
80+
- Do not hesitate to give positive reviews on a series from your
81+
work colleague. If your positive review is written well, it will
82+
not make you look as if you two are representing corporate
83+
interest on a series that is otherwise uninteresting to other
84+
community members and shoving it down their throat.
85+
86+
- Write a positive review in such a way that others can understand
87+
why you support the goal, the approach, and the implementation the
88+
patches took. Make sure to demonstrate that you did thoroughly read
89+
the series and understood problem area well enough to be able to
90+
say that the patches are written well. Feel free to "think out
91+
loud" in your review: describe how you read & understood a complex section of
92+
a patch, ask a question about something that confused you, point out something
93+
you found exceptionally well-written, etc.
94+
95+
- In particular, uplifting feedback goes a long way towards
96+
encouraging contributors to participate more actively in the Git
97+
community.
98+
99+
==== Performing your review
100+
- Provide your review comments per-patch in a plaintext "Reply-All" email to the
101+
relevant patch. Comments should be made inline, immediately below the relevant
102+
section(s).
103+
104+
- You may find that the limited context provided in the patch diff is sometimes
105+
insufficient for a thorough review. In such cases, you can review patches in
106+
your local tree by either applying patches with linkgit:git-am[1] or checking
107+
out the associated branch from https://github.com/gitster/git once the series
108+
is tracked there.
109+
110+
- Large, complicated patch diffs are sometimes unavoidable, such as when they
111+
refactor existing code. If you find such a patch difficult to parse, try
112+
reviewing the diff produced with the `--color-moved` and/or
113+
`--ignore-space-change` options.
114+
115+
- If a patch is long, you are encouraged to delete parts of it that are
116+
unrelated to your review from the email reply. Make sure to leave enough
117+
context for readers to understand your comments!
118+
119+
- If you cannot complete a full review of a series all at once, consider letting
120+
the author know (on- or off-list) if/when you plan to review the rest of the
121+
series.
122+
123+
Completing a review
124+
~~~~~~~~~~~~~~~~~~~
125+
Once each patch of a series is reviewed, the author (and/or other contributors)
126+
may discuss the review(s). This may result in no changes being applied, or the
127+
author will send a new version of their patch(es).
128+
129+
After a series is rerolled in response to your or others' review, make sure to
130+
re-review the updates. If you are happy with the state of the patch series,
131+
explicitly indicate your approval (typically with a reply to the latest
132+
version's cover letter). Optionally, you can let the author know that they can
133+
add a "Reviewed-by: <you>" trailer if they resubmit the reviewed patch verbatim
134+
in a later iteration of the series.
135+
136+
Finally, subsequent "What's cooking" emails may explicitly ask whether a
137+
reviewed topic is ready for merging to the `next` branch (typically phrased
138+
"Will merge to \'next\'?"). You can help the maintainer and author by responding
139+
with a short description of the state of your (and others', if applicable)
140+
review, including the links to the relevant thread(s).
141+
142+
Terminology
143+
-----------
144+
nit: ::
145+
Denotes a small issue that should be fixed, such as a typographical error
146+
or misalignment of conditions in an `if()` statement.
147+
148+
aside: ::
149+
optional: ::
150+
non-blocking: ::
151+
Indicates to the reader that the following comment should not block the
152+
acceptance of the patch or series. These are typically recommendations
153+
related to code organization & style, or musings about topics related to
154+
the patch in question, but beyond its scope.
155+
156+
s/<before>/<after>/::
157+
Shorthand for "you wrote <before>, but I think you meant <after>," usually
158+
for misspellings or other typographical errors. The syntax is a reference
159+
to "substitute" command commonly found in Unix tools such as `ed`, `sed`,
160+
`vim`, and `perl`.
161+
162+
cover letter::
163+
The "Patch 0" of a multi-patch series. This email describes the
164+
high-level intent and structure of the patch series to readers on the
165+
Git mailing list. It is also where the changelog notes and range-diff of
166+
subsequent versions are provided by the author.
167+
+
168+
On single-patch submissions, cover letter content is typically not sent as a
169+
separate email. Instead, it is inserted between the end of the patch's commit
170+
message (after the `---`) and the beginning of the diff.
171+
172+
#leftoverbits::
173+
Used by either an author or a reviewer to describe features or suggested
174+
changes that are out-of-scope of a given patch or series, but are relevant
175+
to the topic for the sake of discussion.
176+
177+
See Also
178+
--------
179+
link:/docs/MyFirstContribution[MyFirstContribution]

0 commit comments

Comments
 (0)