DOC: Cleanup/restructure PR guidelines

Author: timhoffm

doc/devel/pr_guide.rst

@@ -111,7 +111,7 @@ Workflow
 * Set the :ref:`milestone <pr-milestones>`.
 * Keep an eye on the :ref:`number of commits <pr-squashing>`.
 * Approve if all of the above topics are handled.
-* :ref:`Merge  <pr-merging>` if a sufficient number of approvals is reached.
+* :ref:`Merge <pr-merging>` if a :ref:`sufficient number of approvals <pr-approval>` is reached.
 
 .. _pr-guidelines-details:
 
@@ -190,10 +190,27 @@ All Pull Requests should target the main branch. The milestone tag triggers
 an :ref:`automatic backport <automated-backports>` for milestones which have
 a corresponding branch.
 
-.. _pr-merging:
+.. _pr-review:
 
-Merging
--------
+Review
+------
+
+* Do not let perfect be the enemy of the good, particularly for
+  documentation or example PRs.  If you find yourself making many
+  small suggestions, either open a PR against the original branch,
+  push changes to the contributor branch, or merge the PR and then
+  open a new PR against upstream.
+
+* If you push to a contributor branch leave a comment explaining what
+  you did, ex "I took the liberty of pushing a small clean-up PR to
+  your branch, thanks for your work.".  If you are going to make
+  substantial changes to the code or intent of the PR please check
+  with the contributor first.
+
+.. _pr-approval:
+
+Approval
+--------
 As a guiding principle, we require two `approvals`_ from core developers (those
 with commit rights) before merging a pull request. This two-pairs-of-eyes
 strategy shall ensure a consistent project direction and prevent accidental
@@ -229,18 +246,22 @@ Some explicit rules following from this:
     A core dev should only champion one PR at a time and we should try to keep
     the flow of championed PRs reasonable.
 
-After giving the last required approval, the author of the approval should
-merge the PR. PR authors should not self-merge except for when another reviewer
-explicitly allows it (e.g., "Approve modulo CI passing, may self merge when
-green", or "Take or leave the comments. You may self merge".).
-
 .. _pr-automated-tests:
 
 Automated tests
 ---------------
 Before being merged, a PR should pass the :ref:`automated-tests`. If you are
 unsure why a test is failing, ask on the PR or in our :ref:`communication-channels`
 
+.. _pr-merging:
+
+Merging
+-------
+After giving the last required :ref:`approval <pr-approval>`, the author of the
+approval should merge the PR. PR authors should not self-merge except for when
+another reviewer explicitly allows it (e.g., "Approve modulo CI passing, may
+self-merge when green", or "Take or leave the comments. You may self merge".).
+
 .. _pr-squashing:
 
 Number of commits and squashing
@@ -252,19 +273,6 @@ Number of commits and squashing
   about it is to eliminate binary files (ex multiple test image
   re-generations) and to remove upstream merges.
 
-* Do not let perfect be the enemy of the good, particularly for
-  documentation or example PRs.  If you find yourself making many
-  small suggestions, either open a PR against the original branch,
-  push changes to the contributor branch, or merge the PR and then
-  open a new PR against upstream.
-
-* If you push to a contributor branch leave a comment explaining what
-  you did, ex "I took the liberty of pushing a small clean-up PR to
-  your branch, thanks for your work.".  If you are going to make
-  substantial changes to the code or intent of the PR please check
-  with the contributor first.
-
-
 .. _branches_and_backports:
 
 Branches and backports