[PATCH] D100714: Add a new tutorial that talk about how to make a change to llvm.

Mon Apr 19 06:55:11 PDT 2021

xgupta added inline comments.

================
Comment at: llvm/docs/MyFirstTypoFix.rst:60
+   version <https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library>`__
+   of clang, gcc, or Visual Studio.
+
----------------
nit - clang -> Clang, gcc -> GCC

================
Comment at: llvm/docs/MyFirstTypoFix.rst:116
+
+First, create a directory to build in. Usually this is
+llvm-project/build.
----------------
nit - a comma after Usually

================
Comment at: llvm/docs/MyFirstTypoFix.rst:188
+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:
----------------
nit - a comma after `In ninja` and space before `If`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:197
+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:
----------------
nit - space before `When`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:231
+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
----------------
nit - space after `in this case`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:232
+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:
----------------
nit - space before `Let's`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:289
+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:
----------------
space before `Instead`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:290
+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:
+
----------------
nit - space before `There'

================
Comment at: llvm/docs/MyFirstTypoFix.rst:370
+under the "subscribers" section. It should print a code review URL:
+https://reviews.llvm.org/D58291You can always find your active reviews
+on Phabricator under "My activity".
----------------
space is needed after D58291

================
Comment at: llvm/docs/MyFirstTypoFix.rst:379
+When you upload a change for review, an email is sent to you, the
+cfe-commits list, and anyone else subscribed to these kind of changes.
+Within a few days, someone should start the review. They may add
----------------
kind -> kinds

================
Comment at: llvm/docs/MyFirstTypoFix.rst:392
+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
----------------
space before `Note`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:427
+yourself yet.The reviewer **doesn't know this**, so you need to tell
+them!Leave a message on the review like:
+
----------------
space before `The reviewer` and `Leave`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:458
+   in a better position to judge; if this feels like it's not the right
+   option, you can contact the -dev mailing list to get more feedback on
+   the direction;
----------------
-dev is not clear I think, maybe `cfe-dev` or `respective dev` understood better.

================
Comment at: llvm/docs/MyFirstTypoFix.rst:483
+simplest if the username/email are the same as your Phabricator account,
+but these are two separate things.The details are in the `developer
+policy
----------------
space before `The`

================
Comment at: llvm/docs/MyFirstTypoFix.rst:493
+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
----------------
space before `At` missing.

================
Comment at: llvm/docs/MyFirstTypoFix.rst:521
+
+At this point git log should show a single commit on top of
+origin/master.
----------------
`git show` is recommended here, see - https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator

================
Comment at: llvm/docs/MyFirstTypoFix.rst:528
+
+   $ ../llvm/utils/git-svn/git-llvm push
+
----------------
Again `git push https://github.com/llvm/llvm-project.git HEAD:main` recommended here see above doc.

================
Comment at: llvm/docs/MyFirstTypoFix.rst:530
+
+Behind the scenes, this uses svn to actually land the change.You should
+see your change `on
----------------
space before `You`

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100714/new/

https://reviews.llvm.org/D100714