Skip to content

Commit dfc4065

Browse files
[llvm][docs] Clean up the "Landing Your Change" section of the GitHub docs (#112869)
* Note up front that the author may not have permissions to use the merge button and should ask a reviewer to do those steps. * Make it clear that a single commit PR can be landed with a single button click. * There are in fact 3 ways to land a multi-commit PR. * Order the ways in increasing amount of overhead for the PR author. * Put them in bullet point sections so they are visually separate. * Add a note that force pushes can be problematic when the PR has multiple authors, but don't go too much into how to solve that, Git's docs are better here anyway.
1 parent ad70f3e commit dfc4065

File tree

1 file changed

+58
-42
lines changed

1 file changed

+58
-42
lines changed

llvm/docs/GitHub.rst

Lines changed: 58 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -133,64 +133,80 @@ or in some dependent code.
133133
After your PR is reviewed and accepted, you want to rebase your branch to ensure
134134
you won't encounter merge conflicts when landing the PR.
135135

136+
.. note::
137+
This guide assumes that the PR branch only has 1 author. If you are
138+
collaborating with others on a single branch, be careful how and when you push
139+
changes. ``--force-with-lease`` may be useful in this situation.
140+
136141
Landing your change
137142
-------------------
138-
When your PR has been accepted you can use the web interface to land your change.
139-
If you have created multiple commits to address feedback at this point you need
140-
to consolidate those commits into one commit. There are two different ways to
141-
do this:
142143

143-
`Interactive rebase <https://git-scm.com/docs/git-rebase#_interactive_mode>`_
144-
with fixup's. This is the recommended method since you can control the final
145-
commit message and inspect that the final commit looks as you expect. When
146-
your local state is correct, remember to force-push to your branch and press
147-
the merge button afterwards.
144+
When your PR has been accepted you can merge your changes.
148145

149-
Use the button `Squash and merge` in GitHub's web interface, if you do this
150-
remember to review the commit message when prompted.
146+
If you do not have write permissions for the repository, the merge button in
147+
GitHub's web interface will be disabled. If this is the case, continue following
148+
the steps here but ask one of your reviewers to click the merge button on your
149+
behalf.
151150

152-
Afterwards you can select the option `Delete branch` to delete the branch
153-
from your fork.
151+
If the PR is a single commit, all you need to do is click the merge button in
152+
GitHub's web interface.
154153

155-
You can also merge via the CLI by switching to your branch locally and run:
154+
If your PR contains multiple commits, you need to consolidate those commits into
155+
one commit. There are three different ways to do this, shown here with the most
156+
commonly used first:
156157

157-
::
158+
* Use the button `Squash and merge` in GitHub's web interface, if you do this
159+
remember to review the commit message when prompted.
158160

159-
gh pr merge --squash --delete-branch
161+
Afterwards you can select the option `Delete branch` to delete the branch
162+
from your fork.
160163

161-
If you observe an error message from the above informing you that your pull
162-
request is not mergeable, then that is likely because upstream has been
163-
modified since your pull request was authored in a way that now results in a
164-
merge conflict. You must first resolve this merge conflict in order to merge
165-
your pull request. In order to do that:
164+
* `Interactive rebase <https://git-scm.com/docs/git-rebase#_interactive_mode>`_
165+
with fixups. This is the recommended method since you can control the final
166+
commit message and check that the final commit looks as you expect. When
167+
your local state is correct, remember to force-push to your branch and press
168+
the merge button in GitHub's web interface afterwards.
166169

167-
::
170+
* Merge using the GitHub command line interface. Switch to your branch locally
171+
and run:
168172

169-
git fetch upstream
170-
git rebase upstream/main
173+
::
171174

172-
Then fix the source files causing merge conflicts and make sure to rebuild and
173-
retest the result. Then:
175+
gh pr merge --squash --delete-branch
174176

175-
::
177+
If you observe an error message from the above informing you that your pull
178+
request is not mergeable, then that is likely because upstream has been
179+
modified since your pull request was authored in a way that now results in a
180+
merge conflict. You must first resolve this merge conflict in order to merge
181+
your pull request. In order to do that:
176182

177-
git add <files with resolved merge conflicts>
178-
git rebase --continue
183+
::
179184

180-
Finally, you'll need to force push to your branch one more time before you can
181-
merge:
185+
git fetch upstream
186+
git rebase upstream/main
182187

183-
::
188+
Then fix the source files causing merge conflicts and make sure to rebuild and
189+
retest the result. Then:
184190

185-
git push -f
186-
gh pr merge --squash --delete-branch
191+
::
192+
193+
git add <files with resolved merge conflicts>
194+
git rebase --continue
195+
196+
Finally, you'll need to force push to your branch one more time before you can
197+
merge:
198+
199+
::
200+
201+
git push --force
202+
gh pr merge --squash --delete-branch
187203

188-
This force push may ask if you intend to push hundreds, or potentially
189-
thousands of patches (depending on how long it's been since your pull request
190-
was initially authored vs. when you intended to merge it). Since you're pushing
191-
to a branch in your fork, this is ok and expected. Github's UI for the pull
192-
request will understand that you're rebasing just your patches, and display
193-
this result correctly with a note that a force push did occur.
204+
This force push may ask if you intend to push hundreds, or potentially
205+
thousands of patches (depending on how long it's been since your pull request
206+
was initially authored vs. when you intended to merge it). Since you're pushing
207+
to a branch in your fork, this is ok and expected. Github's UI for the pull
208+
request will understand that you're rebasing just your patches, and display
209+
this result correctly with a note that a force push did occur.
194210

195211

196212
Problems After Landing Your Change
@@ -283,7 +299,7 @@ checks:
283299
ninja check
284300

285301
# Push the rebased changes to your fork.
286-
git push origin my_change -f
302+
git push origin my_change --force
287303

288304
# Now merge it
289305
gh pr merge --squash --delete-branch
@@ -378,7 +394,7 @@ checks:
378394
ninja check
379395

380396
# Push the rebased changes to your fork.
381-
git push origin my_change -f
397+
git push origin my_change --force
382398

383399
Once your PR is approved, rebased, and tests are passing, click `Squash and
384400
Merge` on your PR in the GitHub web interface.

0 commit comments

Comments
 (0)