[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