[PATCH] D24167: Moving to GitHub - Unified Proposal

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: docs/Proposals/GitHubMove.rst:149
@@ +148,3 @@
+  TODO: Is this going to work when people push via the SVN bridge?
+
+We'll do this with a combination of client-side and server-side hooks. GitHub
----------------
Right now it is one or the other, I asked the github support to consider adding the option to have SVN commits bypass the status-check, they are considering it (no promises).

================
Comment at: docs/Proposals/GitHubMove.rst:159
@@ +158,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
----------------
rengolin wrote:
> No need, GitHub has email hooks: https://help.github.com/articles/managing-notifications-for-pushes-to-a-repository/
This does not line up with "We'll keep the exact same email format". 

================
Comment at: docs/Proposals/GitHubMove.rst:225
@@ +224,3 @@
+Under the multirepo, things are more involved.  We describe here the proposed
+solution.
+
----------------
rengolin wrote:
> This is a nit, please don't take it personal...
> 
> When I read "things are more involved", I had a negative feeling that "it's complicated".
> 
> Down there, when explaining the "involved" way of checking out a singular repo (compiler-rt), instead, you say "there are a number of options", and I had a positive feeling of "choice".
> 
> Even though they mean the same thing, it felt different. I don't particularly mind either way, but to avoid backlash, I'd try to be consistent and use the same (preferably neutral) phrases for all cases.
They don't mean the same thing. Here it is more complicated. 
The other one exposes multiple options with various tradeoff.

================
Comment at: docs/Proposals/GitHubMove.rst:276
@@ +275,3 @@
+  # or using the GitHub svn native bridge
+  svn co https://github.com/llvm-project/llvm/trunk
+
----------------
rengolin wrote:
> Better to keep the same order svn/git. And don't need to specify that it's a bridge, since you mention above.
The "canonical way" changes. The bridge isn't mentioned in this section, I rather have it clear (and it doesn't hurt). 

================
Comment at: docs/Proposals/GitHubMove.rst:296
@@ +295,3 @@
+
+Commits are performed using `svn commit` or `git commit` and `git svn dcommit`.
+
----------------
rengolin wrote:
> Can you commit via git directly?
How do you use git svn?

================
Comment at: docs/Proposals/GitHubMove.rst:423
@@ +422,3 @@
+  git submodule init
+  git submodule update clang llvm libcxx
+
----------------
rengolin wrote:
> Maybe mention --recursive, too?
What's the point?

================
Comment at: docs/Proposals/GitHubMove.rst:519
@@ +518,3 @@
+
+SVN does not have builtin bisection support. Using the existing Git read-only
+view of the repositories, it is possible to use the native Git bisection script
----------------
probinson wrote:
> SVN bisection is not built-in but it is easy to do manually (or scripted) because you can do `svn update -r $REVISION` to an arbitrary revision.  Because revisions are integers, do `(BAD - GOOD)/2` to pick the next revision.  So, it is not materially harder than bisecting on the multirepo.
Thanks Paul, I tried to clarify, can you double-check?

================
Comment at: docs/Proposals/GitHubMove.rst:573
@@ +572,3 @@
+  be prepared for a one-time change to the revision numbers. The multirepo
+  proposal does not provide a great solution for this.
+
----------------
probinson wrote:
> rengolin wrote:
> > This last phrase is odd... It's not clear what "this" is, but I think you mean "a single repo in build structure".
> > 
> > In a multi-repo, people will continue to checkout independent projects and commit directly to them, there's no difference for them.
> Wouldn't the multirepo still have SVN views on each subproject? Seems like the SVN views would basically be the same for either multirepo or monorepo.
Tried to clarify: the multirepo breaks the cross-project synchronization with SVN.

================
Comment at: docs/Proposals/GitHubMove.rst:616
@@ +615,3 @@
+they aren't forced to download or check out all of llvm, clang, etc. just to
+make a change to libcxx.)
+
----------------
rengolin wrote:
> Can they checkout the read-only libc++ and commit without checking out the entire monorepo?
This is covered in the section `Checkout/Clone a Single Project, with Commit Access` (or I don't understand the question)

================
Comment at: docs/Proposals/GitHubMove.rst:626
@@ +625,3 @@
+
+* Repository: https://github.com/llvm-beanz/llvm-submodules
+* Update bot: http://beanz-bot.com:8180/jenkins/job/submodule-update/
----------------
rengolin wrote:
> This is confusing... I thought you were going to list all of them, mono and multi.
This is the intention, should be updated later.


https://reviews.llvm.org/D24167