[PATCH] D56654: Update GettingStarted guide to recommend that people use the new official Git repository.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

Well this is heroic.



================
Comment at: llvm/docs/GettingStarted.rst:24
 
-There is a third, optional piece called Test Suite.  It is a suite of programs
-with a testing harness that can be used to further test LLVM's functionality
-and performance.
+There's other components as well:
+the `libc++ C++ standard library <https://libcxx.llvm.org>`
----------------
Nit: "There's" stands for "There is", which doesn't fit.  :)


================
Comment at: llvm/docs/GettingStarted.rst:26
+the `libc++ C++ standard library <https://libcxx.llvm.org>`
+the `LLD linker <https://lld.llvm.org>`, and more.
 
----------------
Comma after libc++?


================
Comment at: llvm/docs/GettingStarted.rst:41
 
-   * In particular, the *relative paths specified are important*.
-
 #. Checkout LLVM:
 
----------------
Maybe we could be explicit that this checks out "greater LLVM" rather than "lesser LLVM", especially since this is different from how it used to be.


================
Comment at: llvm/docs/GettingStarted.rst:96
+       ``make -j NNN`` (with an appropriate value of NNN, e.g. number of CPUs
+       you have.)
 
----------------
or just `make -j`?


================
Comment at: llvm/docs/GettingStarted.rst:438
+
+Assume ``origin/master`` points the upstream and ``mybranch`` points your
+working branch, and ``mybranch`` is rebased onto ``origin/master``.  At first
----------------
"points upstream" (or I suppose "points to the upstream repository")


================
Comment at: llvm/docs/GettingStarted.rst:438
+
+Assume ``origin/master`` points the upstream and ``mybranch`` points your
+working branch, and ``mybranch`` is rebased onto ``origin/master``.  At first
----------------
jlebar wrote:
> "points upstream" (or I suppose "points to the upstream repository")
points to your


================
Comment at: llvm/docs/GettingStarted.rst:439
+Assume ``origin/master`` points the upstream and ``mybranch`` points your
+working branch, and ``mybranch`` is rebased onto ``origin/master``.  At first
+you may check sanity of whitespaces:
----------------
s#is rebased onto#is based on origin/master without  merges#?  Like, rebasing is an action, and the result of the action is...that one branch is based on another without merges (or something).


================
Comment at: llvm/docs/GettingStarted.rst:444
 
-  % git diff --check master..mybranch
+  % git diff --check origin/master..mybranch
 
----------------
I wonder if now would be a good point to mention clang-format (and the git-clang-format script) instead?  Like, overall that's probably more useful to introduce to beginners than git diff --check.


================
Comment at: llvm/docs/GettingStarted.rst:582
+|                         | include: clang, libcxx, libcxxabi, libunwind, lldb,|
+|                         | compiler-rt, lld, polly, or debuginfo-tests.       |
++-------------------------+----------------------------------------------------+
----------------
Instead of or in addition to listing them, would it be worthwhile to explain where the canonical list of allowed strings lives (or how to print it)?  That way the list never goes out of date.


================
Comment at: llvm/docs/Phabricator.rst:200
 
   git checkout master
   git merge --ff-only arcpatch-D<Revision>
----------------
I would think that you don't want to merge the change into your local "master" branch, because you probably (?) want that to be a copy of origin/master, and upstream is going to rewrite this change.  IOW `git checkout master; git pull --ff-only origin/master` will fail.

Would it make more sense to checkout `arcpatch-D123`, merge origin/master into it, and then `git llvm push` from that branch?

(I'm also confused why we're talking about patching in a change from phab -- isn't the most likely scenario that you already have the change in your git repository?)


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

https://reviews.llvm.org/D56654