[docs] Add a new tutorial that talk about how to make a change to llvm.

[xzq0528]

Summary

This tutorial will guide you through the process of making a change to LLVM, and contributing it back to the LLVM project.
We'll be making a change to Clang, but the steps for other parts of LLVM are the same.Even though the change we'll be making is simple,
we're going to cover steps like building LLVM, running the tests, and code review. This isgood practice, and you'll be prepared for making larger changes.

Authors: Mikhail Goncharov
Commit: Zhiqian Xia

[kuhnel]

LGTM, thx for converting this!
I guess we should also link to this tutorial from the getting involved page, what do you think?

@meikeb , @gribozavr can you please take a look?

[xgupta]

Very useful tutorial.

[xgupta]

llvm/docs/MyFirstTypoFix.rst

61

nit - clang -> Clang, gcc -> GCC

117

nit - a comma after Usually

189

nit - a comma after In ninja and space before If

198

nit - space before When

232

nit - space after in this case

233

nit - space before Let's

290

space before Instead

291

nit - space before `There'

371

space is needed after D58291

380

kind -> kinds

393

space before Note

428

space before The reviewer and Leave

459

-dev is not clear I think, maybe cfe-dev or respective dev understood better.

484

space before The

494

space before At missing.

522

git show is recommended here, see - https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator

529

Again git push https://github.com/llvm/llvm-project.git HEAD:main recommended here see above doc.

531

space before You

[xgupta]

I tried applying this patch on my local repository, an error message comes during make docs-llvm-html-

Warning, treated as error:
/home/xgupta/llvm-project/llvm/docs/MyFirstTypoFix.rst:document isn't included in any toctree
make[3]: *** [docs/CMakeFiles/docs-llvm-html.dir/build.make:77: docs/CMakeFiles/docs-llvm-html] Error 2
make[2]: *** [CMakeFiles/Makefile2:244554: docs/CMakeFiles/docs-llvm-html.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:244561: docs/CMakeFiles/docs-llvm-html.dir/rule] Error 2
make: *** [Makefile:47070: docs-llvm-html] Error 2

@xzq0528 could you please fix it?

llvm/docs/MyFirstTypoFix.rst

325

Beginners often forget to add a tag like for this patch Zhiqian didn't add [Docs] tag :)
Should be mention here.

[xgupta]

please replace every instance of master to main where appropriate.

llvm/docs/MyFirstTypoFix.rst

518

master is switch to main

[mehdi_amini]

llvm/docs/MyFirstTypoFix.rst

129

For development purpose, I'd advise enabling assertions: -DLLVM_ENABLE_ASSERTIONS=ON.
(this is the default in DEBUG builds, but since you're enabling release build here...)

369–370

This is automatic in Phabricator I believe: the relevant list will be added based on the path in the monorepo.

438

Seems like some other docs should be linked from this docs in the relevant sections, I don't see a ref to https://llvm.org/docs/CodeReview.html or https://llvm.org/docs/CodeReview.html for example.

( But also below in the revert section which like could link https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy )

[mehdi_amini]

llvm/docs/MyFirstTypoFix.rst

325

Is there a reference in an existing doc about the use of tags? I'm not sure we should tell beginners about things that aren't our documented norms.
(I don't use any tag other than "NFC")

325

(I wouldn't see the need to add a tag to this very clear and explicit commit title for example)

[xgupta]

llvm/docs/MyFirstTypoFix.rst

325

So It depends on the person, LLVM Project is large with around 100 commits a day. I don't read the complete line of commit/patch. A tag is useful for me.

So if it is not documented, we can document it. Otherwise, I left this on other reviewers and patch author to decide what is best.

[mehdi_amini]

llvm/docs/MyFirstTypoFix.rst

325

Regardless, this seems to me to be out-of-line for a tutorial to define new conventions.

[dblaikie]

llvm/docs/MyFirstTypoFix.rst

325

Eh, I disagree a bit here - it's the sort of feedback I'd give someone on the mailing list, so might as well write it down here - can be written down other places too.

[mehdi_amini]

llvm/docs/MyFirstTypoFix.rst

325

You're not clear with what you are disagreeing exactly: there is my point that a tutorial should not introduce new conventions, and whether in this case this is a documented convention or something that we've been consistently doing for a while.

We can talk about the latter, but I disagree that this is a settled thing which goes back to my former point: this isn't the revision to settle this.

(to expand a bit, even if this is out-of-scope for this revision I belive: this practice seems like a recent thing people have been doing, and it hasn't been introduced in a coordinated way and without clear guidelines as far as I know. In practice I see this as *noise* and I disagree that this a good use of the commit title limited space. (common guidelines are for git commit titles to be *short*))

[dblaikie]

llvm/docs/MyFirstTypoFix.rst

325

Sorry, to be clear: I disagree that documenting the convention of using tags is inappropriate for this documentation at this time.

Tags have been used in commit descriptions from before the commit message was used in email titles (previously the title was a list of the files touched - so the title gave some sense of what part of the project the patch related to - albeit rather roughly/poorly) - but they're even more useful since that change (~8 years ago), eg: https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130128/163757.html

But sure, if you fundamentally object to it being done in this patch, so be it.

[mehdi_amini]

llvm/docs/MyFirstTypoFix.rst

325

Yeah I fundamentally object to a tutorial introducing recommendation for this while there is no documentation and guidelines for it.
If you want to propose a documentation for a guideline regarding this, why not, but please do as a separate patch and make sure it is documented outside of a tutorial.

[dblaikie]

llvm/docs/MyFirstTypoFix.rst

325

My perspective is that this would be a fine place to include such a suggestion (I don't think as an explicit statement of what you must do, but as a suggestion of some things people do if you think that might clarify things, etc) - in the same way I would in mailing list usage (& have, several times - asking people to flag NFC changes as such as it makes it easier to review). I don't think everything in this document has to be (& I doubt it is, though I haven't read the whole thing) only things that are already formalized in other documentation. (for instance the heuristics mentioned under "Commit access" don't seem to be documented elsewhere - but I don't object to them either, I think it's reasonable to describe to newcomers some norms about how people generally interact in the community))

[kuhnel]

llvm/docs/MyFirstTypoFix.rst

325

Does someone have a (complete) list of tags to be used and their semantics?

[kuhnel]

llvm/docs/MyFirstTypoFix.rst

325

Just looked at the commit history: https://github.com/llvm/llvm-project/commits/main

I can't really see a consistent set of tags people are using. Sometimes it's sub-prrojects (e.g. mlir, clangd) or architectures (ARM, x86) or features or change semantic (NFC). So I guess it would be hard to explain how to pick the right tag.

IMHO the only thing we can say here is that they exist and contributors should try to guess something reasonable...

[nikic]

llvm/docs/MyFirstTypoFix.rst

431

This is missing Please use "My Name <my@email>" to commit the change.

481

This looks outdated?

[dblaikie]

llvm/docs/MyFirstTypoFix.rst

325

It's fairly informal - NFC, target (if it's in a target backend), specific optimization, or general library (ADT, Support, etc) or feature area (DebugInfo) are probably the broad categories.

But I'm not adamant this must be in the document - I'm a bit middle of the road. I don't think missing a tag is necessarily "forgetting" to do something that's required, nor that adding them is problematic or harmful either.

Seems easy enough to leave them out for now if they weren't added - or include it in the example commit message without necessarily any indicator that they must be included or a whole paragraph about them either.

[xgupta]

PIng @xzq0528 for addressing the comments.

[xzq0528]

1. add some space for readability.
2. add some description for tags.
3. update some outdated content: such as origin/master to origin/main, svn tool to git tool.
4. add the link to https://llvm.org/docs/CodeReview.html.

[xgupta]

llvm/docs/MyFirstTypoFix.rst

325

@xzq0528 Actually, here a [Diagnostic] tag would be appropriate, instead of [Docs] one.

[gribozavr2]

llvm/docs/MyFirstTypoFix.rst

3

I'd suggest removing automatically numbered header-ABCD links, I don't think other documents do this. Sphinx will generate a link based on the section title.

[xzq0528]

1. Remove the automatically numbered header.
2. Instead of tags [Docs] with [Diagnostic] at line 324.

[xgupta]

In D100714#2698201, @kuhnel wrote:

I guess we should also link to this tutorial from the getting involved page, what do you think?

@xzq0528 I think you need to address this comment in this patch. because of it following warning/error is generating during build

Warning, treated as error:
/home/xgupta/llvm-project/llvm/docs/MyFirstTypoFix.rst:document isn't included in any toctree
make[3]: *** [docs/CMakeFiles/docs-llvm-html.dir/build.make:77: docs/CMakeFiles/docs-llvm-html] Error 2
make[2]: *** [CMakeFiles/Makefile2:244554: docs/CMakeFiles/docs-llvm-html.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:244561: docs/CMakeFiles/docs-llvm-html.dir/rule] Error 2
make: *** [Makefile:47070: docs-llvm-html] Error 2```.

[xgupta]

llvm/docs/MyFirstTypoFix.rst

325

@xzq0528 Actually, here a [Diagnostic] tag would be appropriate, instead of [Docs] one.

this comment is still not done?

[xgupta]

llvm/docs/MyFirstTypoFix.rst

325

Is there a reference in an existing doc about the use of tags? I'm not sure we should tell beginners about things that aren't our documented norms.
(I don't use any tag other than "NFC")

This [Tag] guideline is documented here: https://llvm.org/docs/DeveloperPolicy.html#commit-messages

[xzq0528]

In D100714#2759101, @xgupta wrote:

In D100714#2698201, @kuhnel wrote:

I guess we should also link to this tutorial from the getting involved page, what do you think?

@xzq0528 I think you need to address this comment in this patch. because of it following warning/error is generating during build

Warning, treated as error:
/home/xgupta/llvm-project/llvm/docs/MyFirstTypoFix.rst:document isn't included in any toctree
make[3]: *** [docs/CMakeFiles/docs-llvm-html.dir/build.make:77: docs/CMakeFiles/docs-llvm-html] Error 2
make[2]: *** [CMakeFiles/Makefile2:244554: docs/CMakeFiles/docs-llvm-html.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:244561: docs/CMakeFiles/docs-llvm-html.dir/rule] Error 2
make: *** [Makefile:47070: docs-llvm-html] Error 2```.

 I will check it as soon as possible.

llvm/docs/MyFirstTypoFix.rst

3

i'll remove it later. Thank you.

325

@xgupta thank you for pointing out the issue.

438

Thanks for your helpful sugeestions, I'll add it later.

481

Thanks for point out the issue.

[xzq0528]

adding the docs into toctree.

[xgupta]

Thank you Zhiqian for updating the revision. But now I am not able to apply patch ( using arc patch D100714) to check the doc build.
Otherwise everything is fine and ready to commit IMO.

[xgupta]

I think you should upload the patch with full context by git diff HEAD~1 -U999999 > mypatch.patch, please see the phabricator documentation.

maybe that was the reason of failed to apply patch error. The error I am getting is

Checking patch llvm/docs/GettingStartedTutorials.rst...
error: while searching for:

GettingStartedVS
ProgrammersManual
tutorial/index

:doc:GettingStarted

Discusses how to get up and running quickly with the LLVM infrastructure.

error: patch failed: llvm/docs/GettingStartedTutorials.rst:12
error: while searching for:

on Windows.

:doc:CompilerWriterInfo

A list of helpful links for compiler writers.

error: patch failed: llvm/docs/GettingStartedTutorials.rst:35
Applied patch llvm/docs/MyFirstTypoFix.rst cleanly.
Applying patch llvm/docs/GettingStartedTutorials.rst with 2 rejects...
Rejected hunk #1.
Rejected hunk #2.

Patch Failed!

llvm/docs/MyFirstTypoFix.rst

170

$ is missing in the beginning.

229

$ is missing in the beginning.

254

$ is missing in the beginning.

275

$ is missing in the beginning.

[xgupta]

@xzq0528 and @meikeb I have send a draft (only did copy) revision for this patch in https://reviews.llvm.org/D108267 and fix the above mention error. If you have no objection we can commit from there as I thought It is a very useful tutorial, should be landed earlier.

[kuhnel]

@xgupta thx for moving forward on this patch.
I fully agree that the tutorial is useful in the current version already and we also improve it after it's publication.

It's probably easier to discuss small modifications after we landed the basic tutorial.

[kwk]

llvm/docs/MyFirstTypoFix.rst

71

@kuhnel ehm, isn't this a bit outdated? I mean we no longer have to use subversion.

[kuhnel]

llvm/docs/MyFirstTypoFix.rst

71

This was fixed already in the current version:
https://llvm.org/docs/MyFirstTypoFix.html

llvm/docs/GettingStartedTutorials.rst

    GettingStartedVS
    ProgrammersManual
    tutorial/index
+   MyFirstTypoFix
 
 :doc:`GettingStarted`
    Discusses how to get up and running quickly with the LLVM infrastructure.

    on Windows.
 
 :doc:`CompilerWriterInfo`
-  A list of helpful links for compiler writers.
+  A list of helpful links for compiler writers.
+
+:doc:`MyFirstTypoFix`
+   This tutorial will guide you through the process of making a change to
+   LLVM, and contributing it back to the LLVM project.
 No newline at end of file

llvm/docs/MyFirstTypoFix.rst

+==============
+MyFirstTypoFix
+==============
+
+.. contents::
+   :local:
+
+Introduction
+============
+
+This tutorial will guide you through the process of making a change to
+LLVM, and contributing it back to the LLVM project. We'll be making a
+change to Clang, but the steps for other parts of LLVM are the same.
+Even though the change we'll be making is simple, we're going to cover
+steps like building LLVM, running the tests, and code review. This is
+good practice, and you'll be prepared for making larger changes.
+
+We'll assume you:
+
+-  know how to use an editor,
+
+-  have basic C++ knowledge,
+
+-  know how to install software on your system,
+
+-  are comfortable with the command line,
+
+-  have basic knowledge of git.
+
+
+The change we're making
+-----------------------
+
+Clang has a warning for infinite recursion:
+
+.. code:: console
+
+   $ echo "void foo() { foo(); }" > ~/test.cc
+   $ clang -c -Wall ~/test.cc
+   input.cc:1:14: warning: all paths through this function will call
+   itself [-Winfinite-recursion]
+
+This is clear enough, but not exactly catchy. Let's improve the wording
+a little:
+
+.. code:: console
+
+   input.cc:1:14: warning: to understand recursion, you must first
+   understand recursion [-Winfinite-recursion]
+
+
+Dependencies
+------------
+
+We're going to need some tools:
+
+-  git: to check out the LLVM source code,
+
+-  a C++ compiler: to compile LLVM source code. You'll want `a recent
+   version <https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library>`__
+   of Clang, GCC, or Visual Studio.
+
+-  CMake: used to configure how LLVM should be built on your system,
+
+-  ninja: runs the C++ compiler to (re)build specific parts of LLVM,
+
+-  python: to run the LLVM tests,
+
+-  arcanist: for uploading changes for review,
+
+-  subversion: to commit changes yourself (later).
+
+As an example, on Ubuntu:
+
+.. code:: console
+
+   $ sudo apt-get install git clang cmake ninja-build python arcanist subversion
+
+
+Building LLVM
+=============
+
+
+Checkout
+--------
+
+The source code is stored `on
+Github <https://github.com/llvm/llvm-project>`__ in one large repository
+("the monorepo").
+
+It may take a while to download!
+
+.. code:: console
+
+   $ git clone https://github.com/llvm/llvm-project.git
+
+This will create a directory "llvm-project" with all of the source
+code.(Checking out anonymously is OK - pushing commits uses a different
+mechanism, as we'll see later)
+
+Configure your workspace
+------------------------
+
+Before we can build the code, we must configure exactly how to build it
+by running CMake. CMake combines information from three sources:
+
+-  explicit choices you make (is this a debug build?)
+
+-  settings detected from your system (where are libraries installed?)
+
+-  project structure (which files are part of 'clang'?)
+
+First, create a directory to build in. Usually, this is
+llvm-project/build.
+
+.. code:: console
+
+   $ mkdir llvm-project/build
+   $ cd llvm-project/build
+
+Now, run CMake:
+
+.. code:: console
+
+   $ cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang
+
+If all goes well, you'll see a lot of "performing test" lines, and
+finally:
+
+.. code:: console
+
+   Configuring done
+   Generating done
+   Build files have been written to: /path/llvm-project/build
+
+And you should see a build.ninja file.
+
+Let's break down that last command a little:
+
+-  **-G Ninja**: we're going to use ninja to build; please create
+   build.ninja
+
+-  **../llvm**: this is the path to the source of the "main" LLVM
+   project
+
+-  The two **-D** flags set CMake variables, which override
+   CMake/project defaults:
+
+-  **CMAKE\ BUILD\ TYPE=Release**: build in optimized mode, which is
+   (surprisingly) the fastest option.
+
+   If you want to run under a debugger, you should use the default Debug
+   (which is totally unoptimized, and will lead to >10x slower test
+   runs) or RelWithDebInfo which is a halfway point.
+   **CMAKE\ BUILD\ TYPE** affects code generation only, assertions are
+   on by default regardless! **LLVM\ ENABLE\ ASSERTIONS=Off** disables
+   them.
+
+-  **LLVM\ ENABLE\ PROJECTS=clang** : this lists the LLVM subprojects
+   you are interested in building, in addition to LLVM itself. Multiple
+   projects can be listed, separated by semicolons, such as "clang;
+   lldb".In this example, we'll be making a change to Clang, so we
+   should build it.
+
+Finally, create a symlink (or a copy) of
+llvm-project/build/compile-commands.json into llvm-project/:
+
+.. code:: console
+
+   ln -s build/compile_commands.json ../
+
+(This isn't strictly necessary for building and testing, but allows
+tools like clang-tidy, clang-query, and clangd to work in your source
+tree).
+
+
+Build and test
+--------------
+
+Finally, we can build the code! It's important to do this first, to
+ensure we're in a good state before making changes. But what to build?
+In ninja, you specify a **target**. If we just want to build the clang
+binary, our target name is "clang" and we run:
+
+.. code:: console
+
+   $ ninja clang
+
+The first time we build will be very slow - Clang + LLVM is a lot of
+code. But incremental builds are fast: ninja will only rebuild the parts
+that have changed. When it finally finishes you should have a working
+clang binary. Try running:
+
+.. code:: console
+
+   $ bin/clang --version
+
+There's also a target for building and running all the clang tests:
+
+.. code:: console
+
+   $ ninja check-clang
+
+This is a common pattern in LLVM: check-llvm is all the checks for core,
+other projects have targets like check-lldb.
+
+
+Making changes
+==============
+
+
+Edit
+----
+
+We need to find the file containing the error message.
+
+.. code:: console
+
+   $ git grep "all paths through this function" ..
+   ../clang/include/clang/Basic/DiagnosticSemaKinds.td:  "all paths through this function will call itself">,
+
+The string that appears in DiagnosticSemaKinds.td is the one that is
+printed by Clang. \*.td files define tables - in this case it's a list
+of warnings and errors clang can emit and their messages. Let's update
+the message in your favorite editor:
+
+.. code:: console
+
+   vi ../clang/include/clang/Basic/DiagnosticSemaKinds.td
+
+Find the message (it should be under
+warn\ *infinite*\ recursive_function)Change the message to "in order to
+understand recursion, you must first understand recursion".
+
+
+Test again
+----------
+
+To verify our change, we can build clang and manually check that it
+works.
+
+.. code:: console
+
+   $ ninja clang
+   $ bin/clang -Wall ~/test.cc
+
+   **/path/test.cc:1:124:** **warning****: in order to understand recursion, you must
+   first understand recursion [-Winfinite-recursion]**
+
+We should also run the tests to make sure we didn't break something.
+
+.. code:: console
+
+   ninja check-clang
+
+Notice that it is much faster to build this time, but the tests take
+just as long to run. Ninja doesn't know which tests might be affected,
+so it runs them all.
+
+.. code:: console
+
+   ********************
+   Testing Time: 408.84s
+   ********************
+   Failing Tests (1):
+       Clang :: SemaCXX/warn-infinite-recursion.cpp
+
+Well, that makes sense… and the test output suggests it's looking for
+the old string "call itself" and finding our new message instead.
+
+Let's fix it by updating the expectation in the test.
+
+.. code:: console
+
+   vi ../clang/test/SemaCXX/warn-infinite-recursion.cpp
+
+Everywhere we see // expected-warning{{call itself}}, let's replace it
+with // expected-warning{{to understand recursion}}.
+
+Now we could run **all** the tests again, but this is a slow way to
+iterate on a change! Instead, let's find a way to re-run just the
+specific test. There are two main types of tests in LLVM:
+
+-  **lit tests** (e.g. SemaCXX/warn-infinite-recursion.cpp).
+
+These are fancy shell scripts that run command-line tools and verify the
+output. They live in files like
+clang/**test**/FixIt/dereference-addressof.c. Re-run like this:
+
+.. code:: console
+
+   $ bin/llvm-lit -v ../clang/test/SemaCXX/warn-infinite-recursion.cpp
+
+-  **unit tests** (e.g. ToolingTests/ReplacementText.CanDeleteAllText)
+
+These are C++ programs that call LLVM functions and verify the results.
+They live in suites like ToolingTests. Re-run like this:
+
+.. code:: console
+
+   $ ninja ToolingTests && tools/clang/unittests/Tooling/ToolingTests
+   --gtest_filter=ReplacementText.CanDeleteAllText
+
+
+Commit locally
+--------------
+
+We'll save the change to a local git branch. This lets us work on other
+things while the change is being reviewed. Changes should have a
+description, to explain to reviewers and future readers of the code why
+the change was made.
+
+.. code:: console
+
+   $ git checkout -b myfirstpatch
+   $ git commit -am "[Diagnostic] Clarify -Winfinite-recursion message"
+
+Now we're ready to send this change out into the world! By the way,
+There is a unwritten convention of using tag for your commit. Tags
+usually represent modules that you intend to modify. If you don't know
+the tags for your modules, you can look at the commit history :
+https://github.com/llvm/llvm-project/commits/main.
+
+
+Code review
+===========
+
+
+Finding a reviewer
+------------------
+
+Changes can be reviewed by anyone in the LLVM community who has commit
+access.For larger and more complicated changes, it's important that the
+reviewer has experience with the area of LLVM and knows the design goals
+well. The author of a change will often assign a specific reviewer (git
+blame and git log can be useful to find one).
+
+As our change is fairly simple, we'll add the cfe-commits mailing list
+as a subscriber; anyone who works on clang can likely pick up the
+review. (For changes outside clang, llvm-commits is the usual list. See
+`http://lists.llvm.org/ <http://lists.llvm.org/mailman/listinfo>`__ for
+all the \*-commits mailing lists).
+
+
+Uploading a change for review
+-----------------------------
+
+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
+"Register new account").
+
+Now you can upload your change for review:
+
+.. code:: console
+
+   $ arc diff HEAD^
+
+This creates a review for your change, comparing your current commit
+with the previous commit. You will be prompted to fill in the review
+details. Your commit message is already there, so just add cfe-commits
+under the "subscribers" section. It should print a code review URL:
+https://reviews.llvm.org/D58291 You can always find your active reviews
+on Phabricator under "My activity".
+
+
+Review process
+--------------
+
+When you upload a change for review, an email is sent to you, the
+cfe-commits list, and anyone else subscribed to these kinds of changes.
+Within a few days, someone should start the review. They may add
+themselves as a reviewer, or simply start leaving comments. You'll get
+another email any time the review is updated. The details are in the
+`https://llvm.org/docs/CodeReview/ <https://llvm.org/docs/CodeReview.html>`__.
+
+
+Comments
+~~~~~~~~
+
+The reviewer can leave comments on the change, and you can reply. Some
+comments are attached to specific lines, and appear interleaved with the
+code. You can either reply to these, or address them and mark them as
+"done". Note that in-line replies are **not** sent straight away! They
+become "draft" comments and you must click "Submit" at the bottom of the
+page.
+
+
+Updating your change
+~~~~~~~~~~~~~~~~~~~~
+
+If you make changes in response to a reviewer's comments, simply run
+
+.. code:: console
+
+   $ arc diff
+
+again to update the change and notify the reviewer. Typically this is a
+good time to send any draft comments as well.
+
+
+Accepting a revision
+~~~~~~~~~~~~~~~~~~~~
+
+When the reviewer is happy with the change, they will **Accept** the
+revision. They may leave some more minor comments that you should
+address, but at this point the review is complete. It's time to get it
+committed!
+
+
+Commit by proxy
+---------------
+
+As this is your first change, you won't have access to commit it
+yourself yet. The reviewer **doesn't know this**, so you need to tell
+them! Leave a message on the review like:
+
+   Thanks @somellvmdev. I don't have commit access, can you land this
+   patch for me? Please use "My Name my@email" to commit the change.
+
+The review will be updated when the change is committed.
+
+
+Review expectations
+-------------------
+
+In order to make LLVM a long-term sustainable effort, code needs to be
+maintainable and well tested. Code reviews help to achieve that goal.
+Especially for new contributors, that often means many rounds of reviews
+and push-back on design decisions that do not fit well within the
+overall architecture of the project.
+
+For your first patches, this means:
+
+-  be kind, and expect reviewers to be kind in return - LLVM has a `Code
+   of Conduct <https://llvm.org/docs/CodeOfConduct.html>`__;
+
+-  be patient - understanding how a new feature fits into the
+   architecture of the project is often a time consuming effort, and
+   people have to juggle this with other responsibilities in their
+   lives; **ping the review once a week** when there is no response;
+
+-  if you can't agree, generally the best way is to do what the reviewer
+   asks; we optimize for readability of the code, which the reviewer is
+   in a better position to judge; if this feels like it's not the right
+   option, you can contact the cfe-dev mailing list to get more feedback
+   on the direction;
+
+
+Commit access
+=============
+
+Once you've contributed a handful of patches to LLVM, start to think
+about getting commit access yourself. It's probably a good idea if:
+
+-  you've landed 3-5 patches of larger scope than "fix a typo"
+
+-  you'd be willing to review changes that are closely related to yours
+
+-  you'd like to keep contributing to LLVM.
+
+
+Getting commit access
+---------------------
+
+LLVM uses Git for committing changes. The details are in the `developer
+policy
+document <https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access>`__.
+
+
+With great power
+----------------
+
+Actually, this would be a great time to read the rest of the `developer
+policy <https://llvm.org/docs/DeveloperPolicy.html>`__, too. At minimum,
+you need to be subscribed to the relevant commits list before landing
+changes (e.g. [e-mail]), as discussion often happens
+there if a new patch causes problems.
+
+
+Commit
+------
+
+Let's say you have a change on a local git branch, reviewed and ready to
+commit. Things to do first:
+
+-  if you used multiple fine-grained commits locally, squash them into a
+   single commit. LLVM prefers commits to match the code that was
+   reviewed. (If you created one commit and then used "arc diff", you're
+   fine)
+
+-  rebase your patch against the latest LLVM code. LLVM uses a linear
+   history, so everything should be based on an up-to-date origin/main.
+
+.. code:: console
+
+   $ git fetch origin/main; git rebase origin/main
+
+-  run the tests one last time, for good luck
+
+At this point git show should show a single commit on top of
+origin/main.
+
+Now you can push your commit with
+
+.. code:: console
+
+   $ git push https://github.com/llvm/llvm-project.git HEAD:main
+
+You should see your change `on
+GitHub <https://github.com/llvm/llvm-project/commits/main>`__ within
+minutes.
+
+
+Post-commit errors
+------------------
+
+Once your change is submitted it will be picked up by automated build
+bots that will build and test your patch in a variety of configurations.
+
+You can see all configurations and their current state in a waterfall
+view at http://lab.llvm.org:8011/waterfall. The waterfall view is good
+to get a general overview over the tested configurations and to see
+which configuration have been broken for a while.
+
+The console view at http://lab.llvm.org:8011/console helps to get a
+better understanding of the build results of a specific patch. If you
+want to follow along how your change is affecting the build bots, **this
+should be the first place to look at** - the colored bubbles correspond
+to projects in the waterfall.
+
+If you see a broken build, do not despair - some build bots are
+continuously broken; if your change broke the build, you will see a red
+bubble in the console view, while an already broken build will show an
+orange bubble. Of course, even when the build was already broken, a new
+change might introduce a hidden new failure.
+
+| When you want to see more details how a specific build is broken,
+  click the red bubble.
+| If post-commit error logs confuse you, do not worry too much -
+  everybody on the project is aware that this is a bit unwieldy, so
+  expect people to jump in and help you understand what's going on!
+
+buildbots, overview of bots, getting error logs.
+
+
+Reverts
+-------
+
+if in doubt, revert and re-land.
+
+
+Conclusion
+==========
+
+llvm is a land of contrasts.