Skip to content

Commit

Permalink
doc: show how to use to git to submit smaller and faster PRs
Browse files Browse the repository at this point in the history
The documentation had for a long time a section that specifically
recommends to submit "smaller PRs" for review:
https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs

Yet submitters keep submitting large PRs on a regular basis, sometimes
very large ones. See a couple of very recent examples below.

(Reminder: submitting a giant, draft PR for pure _testing_ purposes and
NOT for review is a perfectly fine)

The "natural" explanation is that submitters optimistically and wrongly
believe that dumping a large amount of code at once onto reviewers will
be dealt with faster than in smaller chunks. This is most likely a
contributing factor but most people should quickly learn from bad
experience. Yet we keep seeing large PRs on a regular basis. So there
must be other factors too.

Based on personal but fairly extensive git support experience, another
top reason is likely git usability and some lack of git
knowledge (neither the first nor the last time git usability would have
a significant impact)

To help with this, add to the existing git guide the simple command that
lets split a large submission in several, smaller PRs. This can only
help demystify and promote smaller PRs while barely growing the size of
the documentation.

While at it, also add a couple missing benefits of smaller PRs.

Recent examples of large PRs:

- In the controversial and giant PR
#77368 (comment)
the exhausted submitter wrote:

> Every time any one person requests a rebase that changes the PR,
> unless there's consensus, there's no mechanism (manual/project process
> or built into GitHub) to know/prevent a different person from rejecting
> the new changes.

That PR had 21 commits (18 in the final version), 82 files changed and
400 (!) comments. The sheer size massively increased the likelihood of
the problem described.  Re-submitting it in smaller chunks would
obviously have mitigated that problem. Yet that PR was never split and
stayed huge...?

- In this second example, a large PR was submitted with different
authors. A disagreement emerged about squashing across different
authors:
#78795 (comment)
If this PR had been split in smaller chunks, then the squashing
discussion might have been avoided entirely. Whether squashing is good or
bad in this particular case is irrelevant (and already discussed at
great in length in #83117). What matters here is: the submitter lost
that chance by submitting a larger PR with different authors.

Signed-off-by: Marc Herbert <[email protected]>
  • Loading branch information
marc-hb committed Jan 11, 2025
1 parent 8998b1a commit 6cb5b41
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
15 changes: 15 additions & 0 deletions doc/contribute/contributor_expectations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ benefits:

- Easier to revert if the PR breaks functionality.

- More test coverage because CI does not test intermediate commits in a PR;
only PRs as a whole.

- Mixing different authors in the same pull request adds :ref:`extra
complications <modifying_contributions>`.

- When independent of each other, smaller PRs can progress
*concurrently*. Disagreements, nitpicks and other delays in one place
do not hold back everything else (this type of faster, concurrent
workflow is known as "Stacked Diffs").

The :ref:`Contribution workflow` section shows how to use git to submit
several, smaller pull requests. This does not require creating and
managing multiple git branches.

.. note::
This page does not apply to draft PRs which can have any size, any number of
commits and any combination of smaller PRs for testing and preview purposes.
Expand Down
18 changes: 17 additions & 1 deletion doc/contribute/guidelines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,24 @@ workflow here:

git push origin fix_comment_typo

#. Avoid submitting a large number of commits in a single pull request,
see :ref:`contributor-expectations` why. It is tempting to submit
all at once and hope that everything will be merged faster but the
opposite effect is generally achieved. Submitting several, smaller
pull requests does *not* require creating and managing local git
branches::

git push origin big_work~15:refs/heads/big_work_part1

In this example, the 15 most recent commits in the ``big_work`` local
branch are *omitted* from the ``big_work_part1`` remote branch and pull
request. This "Stacked Diffs" technique lets you submit part 1 and get
reviews started long before the rest is ready. When smaller parts are
independent of each other, rotating them with ``git rebase -i`` (see below)
lets you submit them concurrently which is even faster.

#. In your web browser, go to your forked repo and click on the
``Compare & pull request`` button for the branch you just worked on and
``Compare & pull request`` button for the branch you just pushed on and
you want to open a pull request with.

#. Review the pull request changes, and verify that you are opening a pull
Expand Down

0 comments on commit 6cb5b41

Please sign in to comment.