Reviewing Patches in the Git Project
====================================

Introduction
------------
The Git development community is a widely distributed, diverse, ever-changing
group of individuals. Asynchronous communication via the Git mailing list poses
unique challenges when reviewing or discussing patches. This document contains
some guiding principles and helpful tools you can use to make your reviews both
more efficient for yourself and more effective for other contributors.

Note that none of the recommendations here are binding or in any way a
requirement of participation in the Git community. They are provided as a
resource to supplement your skills as a contributor.

Principles
----------

Selecting patch(es) to review
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If you are looking for a patch series in need of review, start by checking
the latest "What's cooking in git.git" email
(https://lore.kernel.org/git/xmqqilm1yp3m.fsf@gitster.g/[example]). The "What's
cooking" emails & replies can be found using the query `s:"What's cooking"` on
the https://lore.kernel.org/git/[`lore.kernel.org` mailing list archive];
alternatively, you can find the contents of the "What's cooking" email tracked
in `whats-cooking.txt` on the `todo` branch of Git. Topics tagged with "Needs
review" and those in the "[New Topics]" section are typically those that would
benefit the most from additional review.

Patches can also be searched manually in the mailing list archive using a query
like `s:"PATCH" -s:"Re:"`. You can browse these results for topics relevant to
your expertise or interest.

If you've already contributed to Git, you may also be CC'd in another
contributor's patch series. These are topics where the author feels that your
attention is warranted. This may be because their patch changes something you
wrote previously (making you a good judge of whether the new approach does or
doesn't work), or because you have the expertise to provide an exceptionally
helpful review. There is no requirement to review these patches but, in the
spirit of open source collaboration, you should strongly consider doing so.

Reviewing patches
~~~~~~~~~~~~~~~~~
While every contributor takes their own approach to reviewing patches, here are
some general pieces of advice to make your reviews as clear and helpful as
possible. The advice is broken into two rough categories: high-level reviewing
guidance, and concrete tips for interacting with patches on the mailing list.

==== High-level guidance
- Remember to review the content of commit messages for correctness and clarity,
  in addition to the code change in the patch's diff. The commit message of a
  patch should accurately and fully explain the code change being made in the
  diff.

- Reviewing test coverage is an important - but easy to overlook - component of
  reviews. A patch's changes may be covered by existing tests, or new tests may
  be introduced to exercise new behavior. Checking out a patch or series locally
  allows you to manually mutate lines of new & existing tests to verify expected
  pass/fail behavior. You can use this information to verify proper coverage or
  to suggest additional tests the author could add.

- When providing a recommendation, be as clear as possible about whether you
  consider it "blocking" (the code would be broken or otherwise made worse if an
  issue isn't fixed) or "non-blocking" (the patch could be made better by taking
  the recommendation, but acceptance of the series does not require it).
  Non-blocking recommendations can be particularly ambiguous when they are
  related to - but outside the scope of - a series ("nice-to-have"s), or when
  they represent only stylistic differences between the author and reviewer.

- When commenting on an issue, try to include suggestions for how the author
  could fix it. This not only helps the author to understand and fix the issue,
  it also deepens and improves your understanding of the topic.

- Reviews do not need to exclusively