diff options
author | Xin Li <delphij@google.com> | 2024-01-17 22:13:58 -0800 |
---|---|---|
committer | Xin Li <delphij@google.com> | 2024-01-17 22:13:58 -0800 |
commit | 28d03a2a1cabbe01d7bcb6cf5166c10e50d3c2c6 (patch) | |
tree | c1643be8ab17fc607cea748a8bb1d621a5964873 /docs/code_reviews.rst | |
parent | ec2628a6ba2d0ecbe3ac10c8c772f6fc6acc345d (diff) | |
parent | f054515492af5132f685cb23fe11891ee77104c9 (diff) | |
download | pigweed-28d03a2a1cabbe01d7bcb6cf5166c10e50d3c2c6.tar.gz |
Merge Android 24Q1 Release (ab/11220357)temp_319669529
Bug: 319669529
Merged-In: Iba357b308a79d0c8b560acd4f72b5423c9c83294
Change-Id: Icdf552029fb97a34e83c6dd7799433fc473a2506
Diffstat (limited to 'docs/code_reviews.rst')
-rw-r--r-- | docs/code_reviews.rst | 132 |
1 files changed, 132 insertions, 0 deletions
diff --git a/docs/code_reviews.rst b/docs/code_reviews.rst new file mode 100644 index 000000000..fcd42082c --- /dev/null +++ b/docs/code_reviews.rst @@ -0,0 +1,132 @@ +.. _docs-code_reviews: + +============ +Code Reviews +============ +All Pigweed development happens on Gerrit, following the `typical Gerrit +development workflow <http://ceres-solver.org/contributing.html>`_. Consult the +`Gerrit User Guide +<https://gerrit-documentation.storage.googleapis.com/Documentation/2.12.3/intro-user.html>`_ +for more information on using Gerrit. + +You may add the special address +``gwsq-pigweed@pigweed.google.com.iam.gserviceaccount.com`` as a reviewer to +have Gerrit automatically choose an appropriate person to review your change. + +----------- +For authors +----------- + +.. _docs-code_reviews-small-changes: + +Small changes +============= +Please follow the guidance in `Google's Eng-Practices Small CLs +<https://google.github.io/eng-practices/review/developer/small-cls.html>`_. + +Complete changes +================ +Please follow the guidance in :ref:`docs-contributing-contribution-standards`. +In summary, CLs must be complete, tested, and include documentation and unit +tests for new code, bug fixes, and any code changes that merit it. However, to +enable iterative work and small changes, ``TODO`` comments are acceptable. They +must include an explanation of the problem and an action to take. + +We will not take over incomplete changes to avoid shifting our focus. We may +reject changes that do not meet the criteria above. + +------------- +For reviewers +------------- + +Review speed +============ +Follow the advice at `Speed of Code Reviews +<https://google.github.io/eng-practices/review/reviewer/speed.html>`_. Most +importantly, + +* If you are not in the middle of a focused task, **you should do a code review + shortly after it comes in**. +* **One business day is the maximum time it should take to respond** to a code + review request (i.e., first thing the next morning). +* If you will not be able to review the change within a business day, comment + on the change stating so, and reassign to another reviewer if possible. + +Attention set management +======================== +Remove yourself from the `attention set +<https://gerrit-review.googlesource.com/Documentation/user-attention-set.html>`_ +for changes where another person (author or reviewer) must take action before +you can continue to review. You are encouraged, but not required, to leave a +comment when doing so, especially for changes by external contributors who may +not be familiar with our process. + +---------------------- +Common advice playbook +---------------------- +What follows are bite-sized copy-paste-able advice when doing code reviews. +Feel free to link to them from code review comments, too. + +.. _docs-code_reviews-playbook-platform-design: + +Shared platforms require careful design +======================================= +Pigweed is a platform shared by many embedded projects. This makes contributing +to Pigweed rewarding: your change will help teams around the world! But it also +makes contributing *hard*: + +* Edge cases that may not matter for one project can, and eventually will, come + up in another one. +* Pigweed has many modules that can be used in isolation, but should also work + together, exhibiting a unified design philosophy and guiding users towards + safe, scalable patterns. + +As a result, Pigweed can't be as nimble as individual embedded projects, and +often needs to engage in more careful design review, either in meetings with +the core team or through :ref:`SEED-0001`. But we're committed to working +through this with you! + + +.. _docs-code_reviews-playbook-stale-changes: + +Stale changes +============= +Sometimes, a change doesn't make it out of the review process: after some +rounds of review, there are unresolved comments from the Pigweed team, but the +author is no longer actively working on the change. + +For any change that's not seen activity for 3 months, the Pigweed team will, + +#. `File a bug <https://issues.pigweed.dev/issues?q=status:open>`_ for the + feature or bug that the change was addressing, referencing the change. +#. Mark the change Abandoned in Gerrit. + +This does *not* mean the change is rejected! It just indicates no further +action on it is expected. As its author, you should feel free to reopen it when +you have time to work on it again. + +Before making or sending major changes or SEEDs, please reach out in our +`chat room <https://discord.gg/M9NSeTA>`_ or on the `mailing list +<https://groups.google.com/forum/#!forum/pigweed>`_ first to ensure the changes +make sense for upstream. We generally go through a design phase before making +large changes. See :ref:`SEED-0001` for a description of this process; but +please discuss with us before writing a full SEED. Let us know of any +priorities, timelines, requirements, and limitations ahead of time. + +Gerrit for PRs +============== +We don't currently support GitHub pull requests. All Pigweed development takes +place on `our Gerrit instance <https://pigweed-review.googlesource.com/>`_. +Please resubmit your change there! + +See :ref:`docs-contributing` for instructions, and consult the `Gerrit User +Guide +<https://gerrit-documentation.storage.googleapis.com/Documentation/2.12.3/intro-user.html>`_ +for more information on using Gerrit. + +.. _docs-code_reviews-incomplete-docs-changes: + +Docs-Only Changes Do Not Need To Be Complete +============================================ +Documentation-only changes should generally be accepted if they make the docs +better or more complete, even if the documentation change itself is incomplete. |