582 lines · plain
1.. _github-reviews:2 3======================4LLVM GitHub User Guide5======================6 7.. contents::8 :local:9 10Introduction11============12The LLVM Project uses `GitHub <https://github.com/>`_ for13`Source Code <https://github.com/llvm/llvm-project>`_,14`Releases <https://github.com/llvm/llvm-project/releases>`_,15`Issue Tracking <https://github.com/llvm/llvm-project/issues>`_., and16`Code Reviews <https://github.com/llvm/llvm-project/pulls>`_.17 18This page describes how the LLVM Project users and developers can19participate in the project using GitHub.20 21Before your first PR22====================23 24Please ensure that you have set a valid email address in your GitHub account,25see :ref:`github-email-address`.26 27Pull Requests28=============29The LLVM project is using GitHub Pull Requests for Code Reviews. This document30describes the typical workflow of creating a Pull Request and getting it reviewed31and accepted. This is meant as an overview of the GitHub workflow, for complete32documentation refer to `GitHub's documentation <https://docs.github.com/pull-requests>`_.33 34.. note::35 If you are using a Pull Request for purposes other than review36 (eg: precommit CI results, convenient web-based reverts, etc)37 add the `skip-precommit-approval <https://github.com/llvm/llvm-project/labels?q=skip-precommit-approval>`_38 label to the PR.39 40GitHub Tools41------------42You can interact with GitHub in several ways: via git command line tools,43the web browser, `GitHub Desktop <https://desktop.github.com/>`_, or the44`GitHub CLI <https://cli.github.com>`_. This guide will cover the git command line45tools and the GitHub CLI.46 47Creating Pull Requests48----------------------49Keep in mind that when creating a pull request, it should generally only contain one50self-contained commit initially.51This makes it easier for reviewers to understand the introduced changes and52provide feedback. It also helps maintain a clear and organized commit history53for the project. If you have multiple changes you want to introduce, it's54recommended to create separate pull requests for each change.55 56Create a local branch per commit you want to submit and then push that branch57to your `fork <https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks>`_58of the llvm-project and59`create a pull request from the fork <https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork>`_.60As GitHub uses the first line of the commit message truncated to 72 characters61as the pull request title, you may have to edit to reword or to undo this62truncation.63 64Creating Pull Requests with GitHub CLI65^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^66With the CLI it's enough to create the branch locally and then run:67 68::69 70 gh pr create71 72When prompted select to create and use your own fork and follow73the instructions to add more information needed.74 75.. note::76 77 When you let the GitHub CLI create a fork of llvm-project to78 your user, it will change the git "remotes" so that "origin" points79 to your fork and "upstream" points to the main llvm-project repository.80 81Updating Pull Requests82----------------------83In order to update your pull request, the only thing you need to do is to push84your new commits to the branch in your fork. That will automatically update85the pull request. You can also use the Update Branch button in GitHub's Pull86Request UI, but be aware that it will create a merge commit on your branch.87 88When updating a pull request, you should push additional "fix up" commits to89your branch instead of force pushing. This makes it easier for GitHub to90track the context of previous review comments. Consider using the91`built-in support for fixups <https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---fixupamendrewordltcommitgt>`_92in git.93 94If you create fix up or merge commits, you must squash and merge before95landing the PR and you must use the pull request title and description as96the commit message. You can do this manually with an interactive git97rebase or with GitHub's built-in tool. See the section about landing your98fix below.99 100When pushing to your branch, make sure you push to the correct fork. Check your101remotes with:102 103::104 105 git remote -v106 107And make sure you push to the remote that's pointing to your fork.108 109Rebasing Pull Requests and Force Pushes110---------------------------------------111In general, you should avoid rebasing a Pull Request and force pushing to the112branch that's the root of the Pull Request during the review. This action will113make the context of the old changes and comments harder to find and read. If114you want to make your pull request up-to-date with main, you might consider115updating your branch, as described in the previous section.116 117Sometimes, a rebase might be needed to update your branch with a fix for a test118or in some dependent code.119 120After your PR is reviewed and accepted, you want to rebase your branch to ensure121you won't encounter merge conflicts when landing the PR.122 123.. note::124 This guide assumes that the PR branch only has 1 author. If you are125 collaborating with others on a single branch, be careful how and when you push126 changes. ``--force-with-lease`` may be useful in this situation.127 128Approvals129---------130 131Before merging a PR you must have the required approvals. See132:ref:`lgtm_how_a_patch_is_accepted` for more details.133 134 135Landing your change136-------------------137 138After your PR is approved, ensure that:139 140 * The PR title and description describe the final changes. These will be used141 as the title and message of the final squashed commit. The titles and142 messages of commits in the PR will **not** be used.143 * You have set a valid email address in your GitHub account, see :ref:`github-email-address`.144 145.. note::146 The LLVM Project monorepo on GitHub is configured to always use "Squash147 and Merge" as the pull request merge option when using the web interface.148 With this option, GitHub uses the PR summary as the default commit149 message.150 151 Users with write access who can merge PRs have a final opportunity to edit152 the commit title and message before merging. However, this option is not153 available to contributors without write access.154 155At this point, you can merge your changes. If you do not have write permissions156for the repository, the merge button in GitHub's web interface will be157disabled. If this is the case, continue following the steps here but ask one of158your reviewers to click the merge button on your behalf.159 160If the PR is a single commit, all you need to do is click the merge button in161GitHub's web interface.162 163If your PR contains multiple commits, you need to consolidate those commits into164one commit. There are three different ways to do this, shown here with the most165commonly used first:166 167* Use the button `Squash and merge` in GitHub's web interface, if you do this168 remember to review the commit message when prompted.169 170 Afterwards you can select the option `Delete branch` to delete the branch171 from your fork.172 173* `Interactive rebase <https://git-scm.com/docs/git-rebase#_interactive_mode>`_174 with fixups. This is the recommended method since you can control the final175 commit message and check that the final commit looks as you expect. When176 your local state is correct, remember to force-push to your branch and press177 the merge button in GitHub's web interface afterwards.178 179* Merge using the GitHub command line interface. Switch to your branch locally180 and run:181 182 ::183 184 gh pr merge --squash --delete-branch185 186 If you observe an error message from the above informing you that your pull187 request is not mergeable, then that is likely because upstream has been188 modified since your pull request was authored in a way that now results in a189 merge conflict. You must first resolve this merge conflict in order to merge190 your pull request. In order to do that:191 192 ::193 194 git fetch upstream195 git rebase upstream/main196 197 Then fix the source files causing merge conflicts and make sure to rebuild and198 retest the result. Then:199 200 ::201 202 git add <files with resolved merge conflicts>203 git rebase --continue204 205 Finally, you'll need to force push to your branch one more time before you can206 merge:207 208 ::209 210 git push --force211 gh pr merge --squash --delete-branch212 213 This force push may ask if you intend to push hundreds, or potentially214 thousands of patches (depending on how long it's been since your pull request215 was initially authored vs. when you intended to merge it). Since you're pushing216 to a branch in your fork, this is ok and expected. Github's UI for the pull217 request will understand that you're rebasing just your patches, and display218 this result correctly with a note that a force push did occur.219 220.. _github_branches:221 222Branches223========224 225It is possible to create branches in `llvm/llvm-project/` that start with226`users/<username>/`, however this is intended to be able to support "stacked"227pull-request. Do not create any branches in the `llvm/llvm-project` repository228otherwise, please use a fork (see above). User branches that aren't229associated with a pull-request **will be deleted**.230 231Stacked Pull Requests232=====================233 234To separate related changes or to break down a larger PR into smaller, reviewable235pieces, use "stacked pull requests" — this helps make the review process236smoother.237 238.. note::239 The LLVM Project monorepo on GitHub is configured to always use "Squash and240 Merge" as the pull request merge option. As a result, each PR results in241 exactly one commit being merged into the project.242 243 This means that stacked pull requests are the only available option for244 landing a series of related changes. In contrast, submitting a PR with245 multiple commits and merging them as-is (without squashing) is not supported246 in LLVM.247 248While GitHub does not natively support stacked pull requests, there are several249common alternatives.250 251To illustrate, assume that you are working on two branches in your fork of the252``llvm/llvm-project`` repository, and you want to eventually merge both into253``main``:254 255- `feature_1`, which contains commit `feature_commit_1`256- `feature_2`, which contains commit `feature_commit_2` and depends on257 `feature_1` (so it also includes `feature_commit_1`)258 259Your options are as follows:260 261#. Use user branches in ``llvm/llvm-project``262 263 Create user branches in the main repository, as described264 :ref:`above<github_branches>`. Then:265 266 - Open a pull request from `users/<username>/feature_1` → `main`267 - Open another from `users/<username>/feature_2` → `users/<username>/feature_1`268 269 This approach allows GitHub to display clean, incremental diffs for each PR270 in the stack, making it much easier for reviewers to see what has changed at271 each step. Once `feature_1` is merged, GitHub will automatically rebase and272 re-target your branch `feature_2` to `main`. For more complex stacks, you can273 perform this step using the web interface.274 275 This approach requires commit access. See how to obtain it276 `here <https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access>`_.277 278#. Two PRs with a dependency note279 280 Create PR_1 for `feature_1` and PR_2 for `feature_2`. In PR_2, include a281 note in the PR summary indicating that it depends on PR_1 (e.g.,282 “Depends on #PR_1”).283 284 To make review easier, make it clear which commits are part of the base PR285 and which are new, e.g. "The first N commits are from the base PR". This286 helps reviewers focus only on the incremental changes.287 288#. Use a stacked PR tool289 290 Use tools like SPR or Graphite (described below) to automate managing291 stacked PRs. These tools are also based on using user branches292 in ``llvm/llvm-project``.293 294.. note::295 When not using user branches, GitHub will not display proper diffs for296 subsequent PRs in a stack. Instead, it will show a combined diff that297 includes all commits from earlier PRs.298 299 As described above, it is the PR author’s responsibility to clearly indicate300 which commits are relevant to the current PR.301 For example: “The first N commits are from the base PR.”302 303 You can avoid this issue by using user branches directly in the304 ``llvm/llvm-project`` repository.305 306 307Using Graphite for stacked Pull Requests308----------------------------------------309 310`Graphite <https://app.graphite.dev/>`_ is a stacked pull request tool supported311by the LLVM repo (the other being `reviewable.io <https://reviewable.io>`_).312 313Graphite will want to create branches under ``llvm/llvm-project`` rather than your314private fork, so the guidance above, about branch naming, is critical, otherwise315``gt submit`` (i.e. publish your PRs for review) will fail.316 317Use ``gt config`` then ``Branch naming settings`` and ``Set a prefix for branch names``.318Include the last ``/``.319 320If you didn't do the above and Graphite created non-prefixed branches, a simple way to321unblock is to rename (``git -m <old name> <new name>``), and then checkout the branch322and ``gt track``.323 324Pre-merge Continuous Integration (CI)325-------------------------------------326 327Multiple checks will be applied on a pull-request, either for linting/formatting328or some build and tests. None of these are perfect and you will encounter329false positive, infrastructure failures (unstable or unavailable worker), or330you will be unlucky and based your change on a broken revision of the main branch.331 332None of the checks are strictly mandatory: these are tools to help us build a333better codebase and be more productive (by avoiding issues found post-merge and334possible reverts). As a developer you're empowered to exercise your judgement335about bypassing any of the checks when merging code.336 337The infrastructure can print messages that make it seem like these are mandatory,338but this is just an artifact of GitHub infrastructure and not a policy of the339project.340 341However, please make sure you do not force-merge any changes that have clear342test failures directly linked to your changes. Our policy is still to keep the343``main`` branch in a good condition, and introducing failures to be fixed later344violates that policy.345 346Problems After Landing Your Change347==================================348 349Even though your PR passed the pre-commit checks and is approved by reviewers, it350may cause problems for some configurations after it lands. You will be notified351if this happens and the community is ready to help you fix the problems.352 353This process is described in detail354:ref:`here <MyFirstTypoFix Issues After Landing Your PR>`.355 356 357Checking out another PR locally358-------------------------------359Sometimes you want to review another person's PR on your local machine to run360tests or inspect code in your preferred editor. This is easily done with the361CLI:362 363::364 365 gh pr checkout <PR Number>366 367This is also possible with the web interface and the normal git command line368tools, but the process is a bit more complicated. See GitHub's369`documentation <https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally?platform=linux&tool=webui#modifying-an-inactive-pull-request-locally>`_370on the topic.371 372Example Pull Request with GitHub CLI373====================================374Here is an example for creating a Pull Request with the GitHub CLI:375 376::377 378 # Clone the repo379 gh repo clone llvm/llvm-project380 381 # Switch to the repo and create a new branch382 cd llvm-project383 git switch -c my_change384 385 # Create your changes386 $EDITOR file.cpp387 388 # Don't forget clang-format389 git clang-format390 391 # and don't forget running your tests392 ninja check-llvm393 394 # Commit, use a good commit message395 git commit file.cpp396 397 # Create the PR, select to use your own fork when prompted.398 # If you don't have a fork, gh will create one for you.399 gh pr create400 401 # If you get any review comments, come back to the branch and402 # adjust them.403 git switch my_change404 $EDITOR file.cpp405 406 # Commit your changes407 git commit file.cpp -m "Code Review adjustments"408 409 # Format changes410 git clang-format HEAD~411 412 # Recommit if any formatting changes413 git commit -a --amend414 415 # Push your changes to your fork branch, be mindful of416 # your remotes here, if you don't remember what points to your417 # fork, use git remote -v to see. Usually origin points to your418 # fork and upstream to llvm/llvm-project419 git push origin my_change420 421Before merging the PR, it is recommended that you rebase locally and re-run test422checks:423 424::425 426 # Add upstream as a remote (if you don't have it already)427 git remote add upstream https://github.com/llvm/llvm-project.git428 429 # Make sure you have all the latest changes430 git fetch upstream && git rebase -i upstream/main431 432 # Make sure tests pass with latest changes and your change433 ninja check434 435 # Push the rebased changes to your fork.436 git push origin my_change --force437 438 # Now merge it439 gh pr merge --squash --delete-branch440 441 442See more in-depth information about how to contribute in the following documentation:443 444* :doc:`Contributing`445* :doc:`MyFirstTypoFix`446 447Example Pull Request with git448====================================449 450Instead of using the GitHub CLI to create a PR, you can push your code to a451remote branch on your fork and create the PR to upstream using the GitHub web452interface.453 454Here is an example of making a PR using git and the GitHub web interface:455 456First follow the instructions to `fork the repository <https://docs.github.com/en/get-started/quickstart/fork-a-repo?tool=webui#forking-a-repository>`_.457 458Next follow the instructions to `clone your forked repository <https://docs.github.com/en/get-started/quickstart/fork-a-repo?tool=webui#cloning-your-forked-repository>`_.459 460Once you've cloned your forked repository,461 462::463 464 # Switch to the forked repo465 cd llvm-project466 467 # Create a new branch468 git switch -c my_change469 470 # Create your changes471 $EDITOR file.cpp472 473 # Don't forget clang-format474 git clang-format475 476 # and don't forget running your tests477 ninja check-llvm478 479 # Commit, use a good commit message480 git commit file.cpp481 482 # Push your changes to your fork branch, be mindful of483 # your remotes here, if you don't remember what points to your484 # fork, use git remote -v to see. Usually origin points to your485 # fork and upstream to llvm/llvm-project486 git push origin my_change487 488Navigate to the URL printed to the console from the git push command in the last step.489Create a pull request from your branch to llvm::main.490 491::492 493 # If you get any review comments, come back to the branch and494 # adjust them.495 git switch my_change496 $EDITOR file.cpp497 498 # Commit your changes499 git commit file.cpp -m "Code Review adjustments"500 501 # Format changes502 git clang-format HEAD~503 504 # Recommit if any formatting changes505 git commit -a --amend506 507 # Re-run tests and make sure nothing broke.508 ninja check509 510 # Push your changes to your fork branch, be mindful of511 # your remotes here, if you don't remember what points to your512 # fork, use git remote -v to see. Usually origin points to your513 # fork and upstream to llvm/llvm-project514 git push origin my_change515 516Before merging the PR, it is recommended that you rebase locally and re-run test517checks:518 519::520 521 # Add upstream as a remote (if you don't have it already)522 git remote add upstream https://github.com/llvm/llvm-project.git523 524 # Make sure you have all the latest changes525 git fetch upstream && git rebase -i upstream/main526 527 # Make sure tests pass with latest changes and your change528 ninja check529 530 # Push the rebased changes to your fork.531 git push origin my_change --force532 533Once your PR is approved, rebased, and tests are passing, click `Squash and534Merge` on your PR in the GitHub web interface.535 536See more in-depth information about how to contribute in the following documentation:537 538* :doc:`Contributing`539* :doc:`MyFirstTypoFix`540 541Releases542========543 544.. _backporting:545 546Backporting Fixes to the Release Branches547-----------------------------------------548You can use special comments on issues or pull requests to make backport549requests for the release branches. To do this, after your pull request has been550merged:551 5521. Edit "Milestone" at the right side of the issue or pull request553 to say "LLVM X.Y Release"554 5552. Add a comment to it in the following format:556 557::558 559 /cherry-pick <commit> <commit> <...>560 561This command takes one or more git commit hashes as arguments and will attempt562to cherry-pick the commit(s) to the release branch. If the commit(s) fail to563apply cleanly, then a comment with a link to the failing job will be added to564the issue/pull request. If the commit(s) do apply cleanly, then a pull request565will be created with the specified commits.566 567If a commit you want to backport does not apply cleanly, you may resolve568the conflicts locally and then create a pull request against the release569branch. Just make sure to add the release milestone to the pull request.570 571Getting admin access to CI infrastructure572=========================================573 574Any individual who is responsible for setting up and/or maintaining CI575infrastructure for a LLVM project can request to be granted the CI/CD role by576the LLVM infrastructure area team. The request can be made by creating `a577Github issue <https://github.com/llvm/llvm-project/issues/new>`_ and using the578``infrastructure`` label. Applicants must include a justification for why the579role is being requested. Applications are reviewed on a case-by-case basis by580the LLVM infrastructure area team and the role can be revoked at any point as581the area team sees fit.582