aboutsummaryrefslogtreecommitdiff
path: root/docs/code_reviews.rst
diff options
context:
space:
mode:
authorXin Li <delphij@google.com>2024-01-17 22:13:58 -0800
committerXin Li <delphij@google.com>2024-01-17 22:13:58 -0800
commit28d03a2a1cabbe01d7bcb6cf5166c10e50d3c2c6 (patch)
treec1643be8ab17fc607cea748a8bb1d621a5964873 /docs/code_reviews.rst
parentec2628a6ba2d0ecbe3ac10c8c772f6fc6acc345d (diff)
parentf054515492af5132f685cb23fe11891ee77104c9 (diff)
downloadpigweed-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.rst132
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.