[llvm] dfc4065 - [llvm][docs] Clean up the "Landing Your Change" section of the GitHub docs (#112869)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 23 01:52:03 PDT 2024
Author: David Spickett
Date: 2024-10-23T09:52:00+01:00
New Revision: dfc40650eccef9ec9a317e7cfe65d9d4fc8fe618
URL: https://github.com/llvm/llvm-project/commit/dfc40650eccef9ec9a317e7cfe65d9d4fc8fe618
DIFF: https://github.com/llvm/llvm-project/commit/dfc40650eccef9ec9a317e7cfe65d9d4fc8fe618.diff
LOG: [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.
Added:
Modified:
llvm/docs/GitHub.rst
Removed:
################################################################################
diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst
index bf6e915d4458f2..ce4308022bf9f0 100644
--- a/llvm/docs/GitHub.rst
+++ b/llvm/docs/GitHub.rst
@@ -133,64 +133,80 @@ or in some dependent code.
After your PR is reviewed and accepted, you want to rebase your branch to ensure
you won't encounter merge conflicts when landing the PR.
+.. note::
+ This guide assumes that the PR branch only has 1 author. If you are
+ collaborating with others on a single branch, be careful how and when you push
+ changes. ``--force-with-lease`` may be useful in this situation.
+
Landing your change
-------------------
-When your PR has been accepted you can use the web interface to land your change.
-If you have created multiple commits to address feedback at this point you need
-to consolidate those commits into one commit. There are two
diff erent ways to
-do this:
-`Interactive rebase <https://git-scm.com/docs/git-rebase#_interactive_mode>`_
-with fixup's. This is the recommended method since you can control the final
-commit message and inspect that the final commit looks as you expect. When
-your local state is correct, remember to force-push to your branch and press
-the merge button afterwards.
+When your PR has been accepted you can merge your changes.
-Use the button `Squash and merge` in GitHub's web interface, if you do this
-remember to review the commit message when prompted.
+If you do not have write permissions for the repository, the merge button in
+GitHub's web interface will be disabled. If this is the case, continue following
+the steps here but ask one of your reviewers to click the merge button on your
+behalf.
-Afterwards you can select the option `Delete branch` to delete the branch
-from your fork.
+If the PR is a single commit, all you need to do is click the merge button in
+GitHub's web interface.
-You can also merge via the CLI by switching to your branch locally and run:
+If your PR contains multiple commits, you need to consolidate those commits into
+one commit. There are three
diff erent ways to do this, shown here with the most
+commonly used first:
-::
+* Use the button `Squash and merge` in GitHub's web interface, if you do this
+ remember to review the commit message when prompted.
- gh pr merge --squash --delete-branch
+ Afterwards you can select the option `Delete branch` to delete the branch
+ from your fork.
-If you observe an error message from the above informing you that your pull
-request is not mergeable, then that is likely because upstream has been
-modified since your pull request was authored in a way that now results in a
-merge conflict. You must first resolve this merge conflict in order to merge
-your pull request. In order to do that:
+* `Interactive rebase <https://git-scm.com/docs/git-rebase#_interactive_mode>`_
+ with fixups. This is the recommended method since you can control the final
+ commit message and check that the final commit looks as you expect. When
+ your local state is correct, remember to force-push to your branch and press
+ the merge button in GitHub's web interface afterwards.
-::
+* Merge using the GitHub command line interface. Switch to your branch locally
+ and run:
- git fetch upstream
- git rebase upstream/main
+ ::
-Then fix the source files causing merge conflicts and make sure to rebuild and
-retest the result. Then:
+ gh pr merge --squash --delete-branch
-::
+ If you observe an error message from the above informing you that your pull
+ request is not mergeable, then that is likely because upstream has been
+ modified since your pull request was authored in a way that now results in a
+ merge conflict. You must first resolve this merge conflict in order to merge
+ your pull request. In order to do that:
- git add <files with resolved merge conflicts>
- git rebase --continue
+ ::
-Finally, you'll need to force push to your branch one more time before you can
-merge:
+ git fetch upstream
+ git rebase upstream/main
-::
+ Then fix the source files causing merge conflicts and make sure to rebuild and
+ retest the result. Then:
- git push -f
- gh pr merge --squash --delete-branch
+ ::
+
+ git add <files with resolved merge conflicts>
+ git rebase --continue
+
+ Finally, you'll need to force push to your branch one more time before you can
+ merge:
+
+ ::
+
+ git push --force
+ gh pr merge --squash --delete-branch
-This force push may ask if you intend to push hundreds, or potentially
-thousands of patches (depending on how long it's been since your pull request
-was initially authored vs. when you intended to merge it). Since you're pushing
-to a branch in your fork, this is ok and expected. Github's UI for the pull
-request will understand that you're rebasing just your patches, and display
-this result correctly with a note that a force push did occur.
+ This force push may ask if you intend to push hundreds, or potentially
+ thousands of patches (depending on how long it's been since your pull request
+ was initially authored vs. when you intended to merge it). Since you're pushing
+ to a branch in your fork, this is ok and expected. Github's UI for the pull
+ request will understand that you're rebasing just your patches, and display
+ this result correctly with a note that a force push did occur.
Problems After Landing Your Change
@@ -283,7 +299,7 @@ checks:
ninja check
# Push the rebased changes to your fork.
- git push origin my_change -f
+ git push origin my_change --force
# Now merge it
gh pr merge --squash --delete-branch
@@ -378,7 +394,7 @@ checks:
ninja check
# Push the rebased changes to your fork.
- git push origin my_change -f
+ git push origin my_change --force
Once your PR is approved, rebased, and tests are passing, click `Squash and
Merge` on your PR in the GitHub web interface.
More information about the llvm-commits
mailing list