Skip to content

Commit 3b29f6d

Browse files
committed
DOC: Cleanup/restructure PR guidelines
- move contents from "Approval" into new section "Merging" - move contents from "Number of commits and squashing" into new section "Review" - Update "Summary for pull request reviewers" accordingly
1 parent 352b419 commit 3b29f6d

File tree

1 file changed

+32
-23
lines changed

1 file changed

+32
-23
lines changed

doc/devel/pr_guide.rst

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ Workflow
109109
* The PR should :ref:`target the main branch <pr-branch-selection>`.
110110
* Tag with descriptive :ref:`labels <pr-labels>`.
111111
* Set the :ref:`milestone <pr-milestones>`.
112-
* Keep an eye on the :ref:`number of commits <pr-squashing>`.
112+
* :ref:`Review <pr-review>` the contents.
113113
* Approve if all of the above topics are handled.
114-
* :ref:`Merge <pr-merging>` if a sufficient number of approvals is reached.
114+
* Keep an eye on the :ref:`number of commits <pr-squashing>`.
115+
* :ref:`Merge <pr-merging>` if a :ref:`sufficient number of approvals <pr-approval>` is reached.
115116

116117
.. _pr-guidelines-details:
117118

@@ -190,10 +191,27 @@ All Pull Requests should target the main branch. The milestone tag triggers
190191
an :ref:`automatic backport <automated-backports>` for milestones which have
191192
a corresponding branch.
192193

193-
.. _pr-merging:
194+
.. _pr-review:
194195

195-
Merging
196-
-------
196+
Review
197+
------
198+
199+
* Do not let perfect be the enemy of the good, particularly for
200+
documentation or example PRs. If you find yourself making many
201+
small suggestions, either open a PR against the original branch,
202+
push changes to the contributor branch, or merge the PR and then
203+
open a new PR against upstream.
204+
205+
* If you push to a contributor branch leave a comment explaining what
206+
you did, ex "I took the liberty of pushing a small clean-up PR to
207+
your branch, thanks for your work.". If you are going to make
208+
substantial changes to the code or intent of the PR please check
209+
with the contributor first.
210+
211+
.. _pr-approval:
212+
213+
Approval
214+
--------
197215
As a guiding principle, we require two `approvals`_ from core developers (those
198216
with commit rights) before merging a pull request. This two-pairs-of-eyes
199217
strategy shall ensure a consistent project direction and prevent accidental
@@ -229,18 +247,22 @@ Some explicit rules following from this:
229247
A core dev should only champion one PR at a time and we should try to keep
230248
the flow of championed PRs reasonable.
231249

232-
After giving the last required approval, the author of the approval should
233-
merge the PR. PR authors should not self-merge except for when another reviewer
234-
explicitly allows it (e.g., "Approve modulo CI passing, may self merge when
235-
green", or "Take or leave the comments. You may self merge".).
236-
237250
.. _pr-automated-tests:
238251

239252
Automated tests
240253
---------------
241254
Before being merged, a PR should pass the :ref:`automated-tests`. If you are
242255
unsure why a test is failing, ask on the PR or in our :ref:`communication-channels`
243256

257+
.. _pr-merging:
258+
259+
Merging
260+
-------
261+
After giving the last required :ref:`approval <pr-approval>`, the author of the
262+
approval should merge the PR. PR authors should not self-merge except for when
263+
another reviewer explicitly allows it (e.g., "Approve modulo CI passing, may
264+
self-merge when green", or "Take or leave the comments. You may self merge".).
265+
244266
.. _pr-squashing:
245267

246268
Number of commits and squashing
@@ -252,19 +274,6 @@ Number of commits and squashing
252274
about it is to eliminate binary files (ex multiple test image
253275
re-generations) and to remove upstream merges.
254276

255-
* Do not let perfect be the enemy of the good, particularly for
256-
documentation or example PRs. If you find yourself making many
257-
small suggestions, either open a PR against the original branch,
258-
push changes to the contributor branch, or merge the PR and then
259-
open a new PR against upstream.
260-
261-
* If you push to a contributor branch leave a comment explaining what
262-
you did, ex "I took the liberty of pushing a small clean-up PR to
263-
your branch, thanks for your work.". If you are going to make
264-
substantial changes to the code or intent of the PR please check
265-
with the contributor first.
266-
267-
268277
.. _branches_and_backports:
269278

270279
Branches and backports

0 commit comments

Comments
 (0)