[PATCH] D24167: Moving to GitHub - Unified Proposal

[Renato Golin via llvm-commits]

rengolin added inline comments.

================
Comment at: docs/Proposals/GitHubMove.rst:160
@@ +159,3 @@
+An extra bot will need to be set up to continue to send emails for every commit.
+We'll keep the exact same email format as we currently have (a change is possible
+later, but beyond the scope of the current discussion), the only difference
----------------
IMO, we don't need to keep the same format, but that's a good point.

Though, it would be better to outline the two options in one quick phrase than leave the other implied. 

================
Comment at: docs/Proposals/GitHubMove.rst:297
@@ +296,3 @@
+
+Commits are performed using `svn commit` or with the sequence `git commit` and
+`git svn dcommit`.
----------------
D'oh, "with the sequence...". Ignore me. 

================
Comment at: docs/Proposals/GitHubMove.rst:424
@@ +423,3 @@
+  git checkout $REVISION
+  git submodule init
+  git submodule update clang llvm libcxx
----------------
"Update" would take one per project and is more cumbersome when you don't know beforehand which or how many projects you'll build (we have that problem). 

Conceptually the same, but recursive gives a better "impression" of simplicity. It's about the bias issue that Chris was talking about, even if totally unintended. 

================
Comment at: docs/Proposals/GitHubMove.rst:574
@@ +573,3 @@
+impacted and have different options.
+
+* If you were pulling from the SVN repo before the switch to Git. The monorepo
----------------
Looks better, thanks! 

================
Comment at: docs/Proposals/GitHubMove.rst:617
@@ +616,3 @@
+the pieces needed for a full toolchain present in one repository. And for
+newcomers, getting and building a toolchain is easier.
+
----------------
That work flow example shows a changed flow for commits, so the statement that "their workflow is unchanged" is not accurate. 

The parentheses comment helps, but doesn't address the issue completely. A better way would be "the workflow is as described in [link] pointing above. 

================
Comment at: docs/Proposals/GitHubMove.rst:627
@@ +626,3 @@
+
+FIXME: make something more official/testable and update all the URLs in the
+examples above.
----------------
Ok 


https://reviews.llvm.org/D24167