-
Notifications
You must be signed in to change notification settings - Fork 7.6k
doc: show how to use to git to submit smaller and faster PRs #83839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -11,14 +11,46 @@ benefits: | |||||
to set aside a few minutes to review smaller changes several times than it is | ||||||
to allocate large blocks of time to review a large PR. | ||||||
|
||||||
- Less wasted work if reviewers or maintainers reject the direction of the | ||||||
change. | ||||||
- Contentious and long discussions have a more restricted "splash damage" | ||||||
because they cannot drown unrelated topics that have successfully escaped | ||||||
in separate Pull Requests. | ||||||
|
||||||
- Easier to rebase and merge. Smaller PRs are less likely to conflict with other | ||||||
changes in the tree. | ||||||
|
||||||
- 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. | ||||||
|
||||||
- Even when smaller PRs are dependent of each other, you can start reviews | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
and getting code merged before the whole work is complete. This faster | ||||||
approach is known as "Stacked Diffs". | ||||||
|
||||||
- Less wasted work if reviewers or maintainers reject the direction of the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how is this considered a "benefit"? If the direction is rejected, and part of it were already merged, that is not considered a good thing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is not new, I merely moved it. I could remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I would remove it honestly. |
||||||
change. | ||||||
|
||||||
- Easier to revert if the PR breaks functionality. | ||||||
|
||||||
- Better test coverage before merge because CI does not test intermediate | ||||||
commits in a PR; only PRs as a whole. | ||||||
|
||||||
- Better test coverage after merge. Pull Request testing is limited | ||||||
because it must provide feedback in a reasonable time. Typically: under | ||||||
one or two hours. After merge, Zephyr-based projects, companies and | ||||||
organizations test the main branch(es) again but for longer and with a | ||||||
much higher level of stress. (Post-merge testing is often called "daily" | ||||||
even when run several times a day.) Merging large changes in smaller | ||||||
chunks provides a tighter and faster feedback loop and makes regressions | ||||||
much more obvious. This is what "Continuous" means in "Continuous | ||||||
Integration". | ||||||
|
||||||
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. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" approach 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. | ||
Comment on lines
+844
to
+855
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would get rid of this. It is in my opinion way too complex for many users to grasp easily. Instead, you could tell people how they can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the very least please don't use |
||
|
||
#. 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 remote branch you just pushed and | ||
you want to open a pull request with. | ||
|
||
#. Review the pull request changes, and verify that you are opening a pull | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rephrase this so that it is simpler to understand, perhaps: