From ceaa087f5cc92eef4e1c84f3177774d9d8f69fba Mon Sep 17 00:00:00 2001 From: Tobias Hieta Date: Fri, 1 Sep 2023 12:28:18 +0200 Subject: [PATCH 1/5] [Doc] Update documentation for the new GitHub workflow This describes the GitHub workflow in the documentation and marks some sections as deprecated. We should clean up these sections ASAP. --- llvm/docs/CodeReview.rst | 12 +-- llvm/docs/Contributing.rst | 44 ++++------- llvm/docs/DeveloperPolicy.rst | 13 ++-- llvm/docs/GettingInvolved.rst | 21 ++--- llvm/docs/GitHub.rst | 142 +++++++++++++++++++++++++++++++--- llvm/docs/MyFirstTypoFix.rst | 6 ++ llvm/docs/Phabricator.rst | 7 ++ llvm/docs/UserGuides.rst | 1 + 8 files changed, 177 insertions(+), 69 deletions(-) diff --git a/llvm/docs/CodeReview.rst b/llvm/docs/CodeReview.rst index 3bd305e93f6fa..5b99f6e3f3f83 100644 --- a/llvm/docs/CodeReview.rst +++ b/llvm/docs/CodeReview.rst @@ -78,10 +78,8 @@ author, for example, might no longer be an active contributor to the project. What Tools Are Used for Code Review? ------------------------------------ -Pre-commit code reviews are conducted on our web-based code-review tool (see -:doc:`Phabricator`). Post-commit reviews can be done on Phabricator, by email -on the relevant project's commit mailing list, on the project's development -list, or on the bug tracker. +Pre-commit code reviews are conducted on GitHub with Pull Requests. See +:ref:`GitHub ` documentation. When Is an RFC Required? ------------------------ @@ -147,12 +145,6 @@ approved. If unsure, the reviewer should provide a qualified approval, (e.g., you are fairly certain that a particular community member will wish to review, even if that person hasn't done so yet. -If you approve of the idea/concept of a patch but feel unqualified to approve, -another option (other than accepting the patch) is to simply *"Award Token"* -(right-hand side in Phabricator) to indicate support without indicating to -other reviewers that the patch has been accepted and reviewed in their -dashboard. - Note that, if a reviewer has requested a particular community member to review, and after a week that community member has yet to respond, feel free to ping the patch (which literally means submitting a comment on the patch with the diff --git a/llvm/docs/Contributing.rst b/llvm/docs/Contributing.rst index 7a47602e3dfc1..aa5bfbfbec7d3 100644 --- a/llvm/docs/Contributing.rst +++ b/llvm/docs/Contributing.rst @@ -88,17 +88,17 @@ in order to update the last commit with all pending changes. the git integration can be run from ``clang/tools/clang-format/git-clang-format``. -We don't currently accept GitHub pull requests, and you'll need to send patches -via :ref:`Phabricator#phabricator-reviews `. -(We used to allow patches on the llvm-commits mailing list, but the mailing lists -have been deprecated.) +The LLVM project is migrating to GitHub Pull Requests as its review process. +We still have an active :ref:`Phabricator ` +instance for the duration of the migration. If you want to contribute to LLVM +now, please use GitHub. For more information about the workflow of using GitHub +Pull Requests see our :ref:`GitHub ` documentation. To make sure the right people see your patch, please select suitable reviewers and add them to your patch when requesting a review. Suitable reviewers are the code owner (see CODE_OWNERS.txt) and other people doing work in the area your -patch touches. If you are using Phabricator, add them to the `Reviewers` field -when creating a review and if you are using `llvm-commits`, add them to the CC of -your email. +patch touches. If you are using GitHub, it will normally suggest some reviewers +based on rules or people that have worked on the code before. A reviewer may request changes or ask questions during the review. If you are uncertain on how to provide test cases, documentation, etc., feel free to ask @@ -111,7 +111,7 @@ on your behalf. If you have received no comments on your patch for a week, you can request a review by 'ping'ing a patch by responding to the email thread containing the -patch, or the Phabricator review with "Ping." The common courtesy 'ping' rate +patch, or the GitHub PR with "Ping." The common courtesy 'ping' rate is once a week. Please remember that you are asking for valuable time from other professional developers. @@ -122,28 +122,9 @@ For more information on LLVM's code-review process, please see :doc:`CodeReview` For developers to commit changes from Git ----------------------------------------- -Once a patch is reviewed, you should rebase it, re-test locally, and commit the -changes to LLVM's main branch. This is done using `git push` if you have the -required access rights. See `committing a change -`_ for Phabricator based commits or -`obtaining commit access `_ -for commit access. - -Here is an example workflow using git. This workflow assumes you have an -accepted commit on the branch named `branch-with-change`. - -.. code-block:: console - - # Pull changes from the upstream main branch. - % git checkout main && git pull - # Rebase your change onto main. - % git rebase --onto main --root branch-with-change - # Rerun the appropriate tests if needed. - % ninja check-$whatever - # Check that the list of commits about to be pushed is correct. - % git log origin/main...HEAD --oneline - # Push to Github. - % git push origin HEAD:main +Once a patch is reviewed, you can select the "Squash and merge" button in the +GitHub web interface. You might need to rebase your change before pushing +it to the repo. LLVM currently has a linear-history policy, which means that merge commits are not allowed. The `llvm-project` repo on github is configured to reject pushes @@ -201,5 +182,6 @@ of LLVM's high-level design, as well as its internals: .. _bug tracker: https://github.com/llvm/llvm-project/issues .. _clang-format-diff.py: https://reviews.llvm.org/source/llvm-github/browse/main/clang/tools/clang-format/clang-format-diff.py .. _git-clang-format: https://reviews.llvm.org/source/llvm-github/browse/main/clang/tools/clang-format/git-clang-format -.. _LLVM's Phabricator: https://reviews.llvm.org/ +.. _LLVM's GitHub: https://github.com/llvm/llvm-project +.. _LLVM's Phabricator (soon deprecated): https://reviews.llvm.org/ .. _LLVM's Open Projects page: https://llvm.org/OpenProjects.html#what diff --git a/llvm/docs/DeveloperPolicy.rst b/llvm/docs/DeveloperPolicy.rst index b51502f138a6b..a453601d7ad93 100644 --- a/llvm/docs/DeveloperPolicy.rst +++ b/llvm/docs/DeveloperPolicy.rst @@ -46,7 +46,7 @@ quality. Stay Informed ------------- -Developers should stay informed by reading the `LLVM Discourse forums`_. +Developers should stay informed by reading the `LLVM Discourse forums`_. If you are doing anything more than just casual work on LLVM, it is suggested that you also subscribe to the "commits" mailing list for the subproject you're interested in, such as `llvm-commits @@ -84,11 +84,8 @@ to read it as possible. As such, we recommend that you: patches may not apply correctly if the underlying code changes between the time the patch was created and the time it is applied. -#. Patches should be unified diffs with "infinite context" (i.e. using something - like `git diff -U999999 main`). - #. Once you have created your patch, create a - `Phabricator review `_ for + :ref:`GitHub Pull Request ` for it (or commit it directly if applicable). When submitting patches, please do not add confidentiality or non-disclosure @@ -116,6 +113,12 @@ diagnostic from a warning to an error, switching important default behavior, or any other potentially disruptive situation thought to be worth raising awareness of. For such changes, the following should be done: +.. warning:: + + Phabricator is deprecated and will be switched to read-only mode in October + 2023, for new code contributions use :ref:`GitHub Pull Requests `. + This section contains old information that needs to be updated. + * When performing the code review for the change, please add any applicable "vendors" group to the review for their awareness. The purpose of these groups is to give vendors early notice that potentially disruptive changes diff --git a/llvm/docs/GettingInvolved.rst b/llvm/docs/GettingInvolved.rst index e79affb286ee5..4ffdcfa2b6d8a 100644 --- a/llvm/docs/GettingInvolved.rst +++ b/llvm/docs/GettingInvolved.rst @@ -14,7 +14,6 @@ LLVM welcomes contributions of all kinds. To get started, please review the foll CodeReview SupportPolicy SphinxQuickstartTemplate - Phabricator HowToSubmitABug BugLifeCycle CodingStandards @@ -38,10 +37,6 @@ LLVM welcomes contributions of all kinds. To get started, please review the foll A template + tutorial for writing new Sphinx documentation. It is meant to be read in source form. -:doc:`Phabricator` - Describes how to use the Phabricator code review tool hosted on - http://reviews.llvm.org/ and its command line interface, Arcanist. - :doc:`HowToSubmitABug` Instructions for properly submitting information about any bugs you run into in the LLVM system. @@ -54,7 +49,7 @@ LLVM welcomes contributions of all kinds. To get started, please review the foll efficient C++ code. :doc:`GitHub` - Describes how to use the llvm-project repository on GitHub. + Describes how to use the llvm-project repository and code reviews on GitHub. :doc:`GitBisecting` Describes how to use ``git bisect`` on LLVM's repository. @@ -110,7 +105,7 @@ Discourse forums. There are also commit mailing lists for all commits to the LLV The :doc:`CodeOfConduct` applies to all these forums and mailing lists. `LLVM Discourse`__ - The forums for all things LLVM and related sub-projects. There are categories and subcategories for a wide variety of areas within LLVM. You can also view tags or search for a specific topic. + The forums for all things LLVM and related sub-projects. There are categories and subcategories for a wide variety of areas within LLVM. You can also view tags or search for a specific topic. .. __: https://discourse.llvm.org/ @@ -130,9 +125,9 @@ The :doc:`CodeOfConduct` applies to all these forums and mailing lists. .. __: http://lists.llvm.org/pipermail/llvm-bugs/ `LLVM Announcements`__ - If you just want project wide announcements such as releases, developers meetings, or blog posts, then you should check out the Announcement category on LLVM Discourse. + If you just want project wide announcements such as releases, developers meetings, or blog posts, then you should check out the Announcement category on LLVM Discourse. - .. __: https://discourse.llvm.org/c/announce/46 + .. __: https://discourse.llvm.org/c/announce/46 .. _online-sync-ups: @@ -146,7 +141,7 @@ The :doc:`CodeOfConduct` applies to all online sync-ups. If you'd like to organize a new sync-up, please add the info in the table below. Please also create a calendar event for it and invite calendar@llvm.org -to the event, so that it'll show up on the :ref:`llvm-community-calendar`. +to the event, so that it'll show up on the :ref:`llvm-community-calendar`. Please see :ref:`llvm-community-calendar-host-guidance` for more guidance on what to add to your calendar invite. @@ -291,7 +286,7 @@ don't find anyone present, chances are they happen to be off that day. - `Video Call `__ - English, German, Spanish, French * - Anastasia Stulova - - Clang internals for C/C++ language extensions and dialects, OpenCL, GPU, SPIR-V, how to contribute, women in compilers. + - Clang internals for C/C++ language extensions and dialects, OpenCL, GPU, SPIR-V, how to contribute, women in compilers. - Monthly, 1st Tuesday of the month at 17:00 BST - London time (9:00am PT except for 2 weeks in spring), 30 mins slot. - `GoogleMeet `__ - English, Russian, German (not fluently) @@ -330,7 +325,7 @@ Guidance for office hours hosts * If you're interested in becoming an office hours host, please add your information to the list above. Please create a calendar event for it and - invite calendar@llvm.org to the event so that it'll show up on the + invite calendar@llvm.org to the event so that it'll show up on the :ref:`llvm-community-calendar`. Please see :ref:`llvm-community-calendar-host-guidance` for more guidance on what to add to your calendar invite. @@ -446,7 +441,7 @@ Guidance on what to put into LLVM community calendar invites To add your event, create a calendar event for it and invite calendar@llvm.org on it. Your event should then show up on the community calendar. - + Please put the following pieces of information in your calendar invite: * Write a single paragraph describing what the event is about. Include things diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst index 0aa117e252fb1..3300325e79e73 100644 --- a/llvm/docs/GitHub.rst +++ b/llvm/docs/GitHub.rst @@ -1,3 +1,5 @@ +.. _github-reviews: + ====================== LLVM GitHub User Guide ====================== @@ -6,8 +8,9 @@ Introduction ============ The LLVM Project uses `GitHub `_ for `Source Code `_, -`Releases `_, and -`Issue Tracking `_. +`Releases `_, +`Issue Tracking `_., and +`Code Reviews `_. This page describes how the LLVM Project users and developers can participate in the project using GitHub. @@ -21,21 +24,45 @@ branches being used for "stacked" pull requests will be allowed. Pull Requests ============= -The LLVM Project does not currently accept pull requests for the llvm/llvm-project -repository. However, there is a -`plan `_ to move -to pull requests in the future. This section documents the pull request -policies LLVM will be adopting once the project starts using them. +The LLVM project is using GitHub Pull Requests for Code Reviews. This document +describes the typical workflow of creating a Pull Request and getting it reviewed +and accepted. This is meant as an overview of the GitHub workflow, for compelete +documentation refer to `GitHubs documentation `_. + +GitHub Tools +------------ +You can interact with GitHub in several ways: via git command line tools, +the web browser, `GitHub Desktop `_, or the +`GitHub CLI `_. This guide will cover the git command line +tools and the GitHub CLI. The GitHub CLI (`gh`) will be most like the `arc` workflow and +recommended. Creating Pull Requests -^^^^^^^^^^^^^^^^^^^^^^ +---------------------- For pull requests, please push a branch to your `fork `_ of the llvm-project and `create a pull request from the fork `_. +Creating Pull Requests with GitHub CLI +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +With the CLI it's enough to create the branch locally and then run: + +:: + + gh pr create + +When promted select to create and use your own fork and follow +the instructions to add more information needed. + +.. note:: + + When you let the GitHub CLI to create a fork of llvm-project to + your user, it will change the git "remotes" so that "origin" points + to your fork and "upstream" points to the main llvm-project repository. + Updating Pull Requests -^^^^^^^^^^^^^^^^^^^^^^ +---------------------- When updating a pull request, you should push additional "fix up" commits to your branch instead of force pushing. This makes it easier for GitHub to track the context of previous review comments. @@ -46,11 +73,106 @@ The default commit message for a squashed pull request is the pull request description, so this will allow reviewers to review the commit message before approving the commit. +When pushing to your branch, make sure you push to the correct fork. Check your +remotes with: + +:: + + git remote -v + +And make sure you push to the remote that's pointing to your fork. + +Rebasing Pull Requests and Force Pushes +--------------------------------------- +In general, you should avoid rebasing a Pull Request and force pushing to the +branch that's the root of the Pull Request during the review. This action will +make the context of the old changes and comments harder to find and read. + +Sometimes, a rebase might be needed to update your branch with a fix for a test +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. + +Landing your change +------------------- +When your PR has been accepted you can use the web interface to land your change. +The button that should be used is called `Squash and merge` and after you can +select the option `Delete branch` to delete the branch from your fork. + +You can also merge via the CLI by switch to your branch locally and run: + +:: + + gh pr merge --squash --delete-branch + + +Checking out another PR locally +------------------------------- +Sometimes you want to review another persons PR on your local machine to run +tests or inspect code in your prefered editor. This is easily done with the +CLI: + +:: + + gh pr checkout + +This is also possible with the web interface and the normal git command line +tools, but the process is a bit more complicated. See GitHubs +`documentation `_ +on the topic. + +Example Pull Request with GitHub CLI +==================================== +Here is an example for creating a Pull Request with the GitHub CLI: + +:: + + # Clone the repo + gh repo clone llvm/llvm-project + + # Switch to the repo and create a new branch + cd llvm-project + git switch -c my_change + + # Create your changes + $EDITOR file.cpp + + # Don't forget clang-format + git clang-format + + # Commit, use a good commit message + git commit file.cpp + + # Create the PR, select to use your own fork when prompted. + gh pr create + + # If you get any review comments, come back to the branch and + # adjust them. + git switch my_change + $EDITOR file.cpp + + # Commit your changes + git commit file.cpp -m "Code Review adjustments" + + # Push your changes to your fork branch, be mindful of + # your remotes here, if you don't remember what points to your + # fork, use git remote -v to see. Usually origin points to your + # fork and upstream to llvm/llvm-project + git push origin my_change + + # When your PR is accepted, you can now rebase it and make sure + # you have all the latest changes. + git rebase -i origin/main + + # Now merge it + gh pr merge --squash --delete + Releases ======== Backporting Fixes to the Release Branches -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +----------------------------------------- You can use special comments on issues to make backport requests for the release branches. This is done by making a comment containing one of the following commands on any issue that has been added to one of the "X.Y.Z Release" diff --git a/llvm/docs/MyFirstTypoFix.rst b/llvm/docs/MyFirstTypoFix.rst index f1ce39a44ac1c..e430dbea7ecdb 100644 --- a/llvm/docs/MyFirstTypoFix.rst +++ b/llvm/docs/MyFirstTypoFix.rst @@ -345,6 +345,12 @@ all the \*-commits mailing lists). Uploading a change for review ----------------------------- +.. warning:: + + Phabricator is deprecated and will be switched to read-only mode in October + 2023, for new code contributions use :ref:`GitHub Pull Requests `. + This section contains old information that needs to be updated. + LLVM code reviews happen at https://reviews.llvm.org. The web interface is called Phabricator, and the code review part is Differential. You should create a user account there for reviews (click "Log In" and then diff --git a/llvm/docs/Phabricator.rst b/llvm/docs/Phabricator.rst index 259175a64ecf1..057e5e0a14070 100644 --- a/llvm/docs/Phabricator.rst +++ b/llvm/docs/Phabricator.rst @@ -4,9 +4,16 @@ Code Reviews with Phabricator ============================= +.. warning:: + + Phabricator is deprecated and will be switched to read-only mode in October + 2023, for new code contributions use :ref:`GitHub Pull Requests `. + + .. contents:: :local: + If you prefer to use a web user interface for code reviews, you can now submit your patches for Clang and LLVM at `LLVM's Phabricator`_ instance. diff --git a/llvm/docs/UserGuides.rst b/llvm/docs/UserGuides.rst index cc5f983b4454c..006df613bc5e7 100644 --- a/llvm/docs/UserGuides.rst +++ b/llvm/docs/UserGuides.rst @@ -34,6 +34,7 @@ intermediate LLVM representation. Docker FatLTO ExtendingLLVM + GitHub GoldPlugin GlobalISel/MIRPatterns HowToBuildOnARM From 13e01d25e7425c361ff3bb372ec09452276c4cda Mon Sep 17 00:00:00 2001 From: Tobias Hieta Date: Fri, 1 Sep 2023 14:53:24 +0200 Subject: [PATCH 2/5] Code review updates. Clarified how to use squash and merge and other smaller fixes. --- llvm/docs/Contributing.rst | 9 ++++--- llvm/docs/GitHub.rst | 46 +++++++++++++++++++++++++----------- llvm/docs/MyFirstTypoFix.rst | 2 +- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/llvm/docs/Contributing.rst b/llvm/docs/Contributing.rst index aa5bfbfbec7d3..d289d7a59c60a 100644 --- a/llvm/docs/Contributing.rst +++ b/llvm/docs/Contributing.rst @@ -97,8 +97,8 @@ Pull Requests see our :ref:`GitHub ` documentation. To make sure the right people see your patch, please select suitable reviewers and add them to your patch when requesting a review. Suitable reviewers are the code owner (see CODE_OWNERS.txt) and other people doing work in the area your -patch touches. If you are using GitHub, it will normally suggest some reviewers -based on rules or people that have worked on the code before. +patch touches. Github will normally suggest some reviewers based on rules or +people that have worked on the code before. A reviewer may request changes or ask questions during the review. If you are uncertain on how to provide test cases, documentation, etc., feel free to ask @@ -110,8 +110,7 @@ access, please let people know during the review and someone should commit it on your behalf. If you have received no comments on your patch for a week, you can request a -review by 'ping'ing a patch by responding to the email thread containing the -patch, or the GitHub PR with "Ping." The common courtesy 'ping' rate +review by 'ping'ing the GitHub PR with "Ping." The common courtesy 'ping' rate is once a week. Please remember that you are asking for valuable time from other professional developers. @@ -183,5 +182,5 @@ of LLVM's high-level design, as well as its internals: .. _clang-format-diff.py: https://reviews.llvm.org/source/llvm-github/browse/main/clang/tools/clang-format/clang-format-diff.py .. _git-clang-format: https://reviews.llvm.org/source/llvm-github/browse/main/clang/tools/clang-format/git-clang-format .. _LLVM's GitHub: https://github.com/llvm/llvm-project -.. _LLVM's Phabricator (soon deprecated): https://reviews.llvm.org/ +.. _LLVM's Phabricator (deprecated): https://reviews.llvm.org/ .. _LLVM's Open Projects page: https://llvm.org/OpenProjects.html#what diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst index 3300325e79e73..42e30bc046d70 100644 --- a/llvm/docs/GitHub.rst +++ b/llvm/docs/GitHub.rst @@ -26,8 +26,8 @@ Pull Requests ============= The LLVM project is using GitHub Pull Requests for Code Reviews. This document describes the typical workflow of creating a Pull Request and getting it reviewed -and accepted. This is meant as an overview of the GitHub workflow, for compelete -documentation refer to `GitHubs documentation `_. +and accepted. This is meant as an overview of the GitHub workflow, for complete +documentation refer to `GitHub's documentation `_. GitHub Tools ------------ @@ -52,26 +52,31 @@ With the CLI it's enough to create the branch locally and then run: gh pr create -When promted select to create and use your own fork and follow +When prompted select to create and use your own fork and follow the instructions to add more information needed. .. note:: - When you let the GitHub CLI to create a fork of llvm-project to + When you let the GitHub CLI create a fork of llvm-project to your user, it will change the git "remotes" so that "origin" points to your fork and "upstream" points to the main llvm-project repository. Updating Pull Requests ---------------------- +In order to update your pull request, the only thing you need to do is to push +your new commits to the branch in your fork. That will automatically update +the pull request. + When updating a pull request, you should push additional "fix up" commits to -your branch instead of force pushing. This makes it easier for GitHub to -track the context of previous review comments. +your branch instead of force pushing. This makes it easier for GitHub to +track the context of previous review comments. Consider using the +`built-in support for fixups `_ +in git. -If you do this, you must squash and merge before committing and +If you do this, you must squash and merge before landing the PR and you must use the pull request title and description as the commit message. -The default commit message for a squashed pull request is the pull request -description, so this will allow reviewers to review the commit message before -approving the commit. +You can do this manually with an interactive git rebase or with GitHub's +built-in tool. See the section about landing your fix below. When pushing to your branch, make sure you push to the correct fork. Check your remotes with: @@ -97,10 +102,23 @@ you won't encounter merge conflicts when landing the PR. Landing your change ------------------- When your PR has been accepted you can use the web interface to land your change. -The button that should be used is called `Squash and merge` and after you can -select the option `Delete branch` to delete the branch from your fork. +If you have created multiple commits to address feedback at this point you need +to consolidate those commits into one commit. There are two different ways to +do this: + +`Interactive rebase `_ +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. + +Use the button `Squash and merge` in GitHub's web interface, if you do this +remember to review the commit message when prompted. + +Afterwards you can select the option `Delete branch` to delete the branch +from your fork. -You can also merge via the CLI by switch to your branch locally and run: +You can also merge via the CLI by switching to your branch locally and run: :: @@ -109,7 +127,7 @@ You can also merge via the CLI by switch to your branch locally and run: Checking out another PR locally ------------------------------- -Sometimes you want to review another persons PR on your local machine to run +Sometimes you want to review another person's PR on your local machine to run tests or inspect code in your prefered editor. This is easily done with the CLI: diff --git a/llvm/docs/MyFirstTypoFix.rst b/llvm/docs/MyFirstTypoFix.rst index e430dbea7ecdb..27324049af330 100644 --- a/llvm/docs/MyFirstTypoFix.rst +++ b/llvm/docs/MyFirstTypoFix.rst @@ -348,7 +348,7 @@ Uploading a change for review .. warning:: Phabricator is deprecated and will be switched to read-only mode in October - 2023, for new code contributions use :ref:`GitHub Pull Requests `. + 2023. For new code contributions use :ref:`GitHub Pull Requests `. This section contains old information that needs to be updated. LLVM code reviews happen at https://reviews.llvm.org. The web interface From 45a213d65e17b1a1fe27dd829bd4b8c0e9d2a56a Mon Sep 17 00:00:00 2001 From: Tobias Hieta Date: Fri, 1 Sep 2023 15:06:22 +0200 Subject: [PATCH 3/5] Another rephrase --- llvm/docs/Contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/docs/Contributing.rst b/llvm/docs/Contributing.rst index d289d7a59c60a..2a032efa663d1 100644 --- a/llvm/docs/Contributing.rst +++ b/llvm/docs/Contributing.rst @@ -88,7 +88,7 @@ in order to update the last commit with all pending changes. the git integration can be run from ``clang/tools/clang-format/git-clang-format``. -The LLVM project is migrating to GitHub Pull Requests as its review process. +The LLVM project has migrated to GitHub Pull Requests as its review process. We still have an active :ref:`Phabricator ` instance for the duration of the migration. If you want to contribute to LLVM now, please use GitHub. For more information about the workflow of using GitHub From 13638857892b5f6ec03da7cbd0bb5a2ac3aed0bb Mon Sep 17 00:00:00 2001 From: Tobias Hieta Date: Fri, 1 Sep 2023 16:23:19 +0200 Subject: [PATCH 4/5] Added reminders about running tests, and links to other relevant documentation --- llvm/docs/GitHub.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst index 42e30bc046d70..80d8d5a4fcfbf 100644 --- a/llvm/docs/GitHub.rst +++ b/llvm/docs/GitHub.rst @@ -159,10 +159,14 @@ Here is an example for creating a Pull Request with the GitHub CLI: # Don't forget clang-format git clang-format + # and don't forget running your tests + ninja check-llvm + # Commit, use a good commit message git commit file.cpp # Create the PR, select to use your own fork when prompted. + # If you don't have a fork, gh will create one for you. gh pr create # If you get any review comments, come back to the branch and @@ -183,9 +187,20 @@ Here is an example for creating a Pull Request with the GitHub CLI: # you have all the latest changes. git rebase -i origin/main + # If this PR is older and you get a lot of new commits with the + # rebase, you might want to re-run tests and make sure nothing + # broke. + ninja check-llvm + # Now merge it gh pr merge --squash --delete + +See more in-depth information about how to contribute in the following documentation: + +* :doc:`Contributing` +* :doc:`MyFirstTypoFix` + Releases ======== From 148e4f79a9b190d034b8d5c6c4df592caed39903 Mon Sep 17 00:00:00 2001 From: Tobias Hieta Date: Fri, 1 Sep 2023 16:41:47 +0200 Subject: [PATCH 5/5] Add paragraph about keeping one pr == one commit --- llvm/docs/GitHub.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst index 80d8d5a4fcfbf..48ddfe515a2ee 100644 --- a/llvm/docs/GitHub.rst +++ b/llvm/docs/GitHub.rst @@ -39,8 +39,14 @@ recommended. Creating Pull Requests ---------------------- -For pull requests, please push a branch to your -`fork `_ +Keep in mind that each pull request should generally only contain one commit. +This makes it easier for reviewers to understand the introduced changes and +provide feedback. It also helps maintain a clear and organized commit history +for the project. If you have multiple changes you want to introduce, it's +recommended to create separate pull requests for each change. + +Create a local branch per commit you want to submit and then push that branch +to your `fork `_ of the llvm-project and `create a pull request from the fork `_.