[PATCH] D24167: Moving to GitHub - Unified Proposal

[Mehdi AMINI via llvm-commits]

mehdi_amini marked 4 inline comments as done.

================
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`.
----------------
rengolin wrote:
> D'oh, "with the sequence...". Ignore me. 
I added "with the sequence" following your comment to make it more clear.


================
Comment at: docs/Proposals/GitHubMove.rst:424
@@ +423,3 @@
+  git checkout $REVISION
+  git submodule init
+  git submodule update clang llvm libcxx
----------------
rengolin wrote:
> "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. 
I'm not sure I follow: AFAIK recursive is for nested submodules, which is not part of the proposal. So to be clear I expect `--recursive` to be a no-op. I can be wrong, but I'll need some more explanation if I missed something obvious here.

If your point is about cloning *all* the sub-projects and not only just a selected list, then `--recursive` is not the right option, just doing `git submodule update` without any other flag will do it.  I'll spell it out.

================
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.
+
----------------
rengolin wrote:
> 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. 
I'm sorry I don't follow. You mention a changed in the flow for commit. Here is what's mentioned in the section I referred to, can you clarify where is the inaccuracy?

Workflow today:

```
  # direct SVN checkout
  svn co https://user@llvm.org/svn/llvm-project/llvm/trunk llvm
  # or using the read-only Git view, with git-svn
  git clone http://llvm.org/git/llvm.git
  cd llvm
  git svn init https://llvm.org/svn/llvm-project/llvm/trunk --username=<username>
  git config svn-remote.svn.fetch :refs/remotes/origin/master
  git svn rebase -l  # -l avoids fetching ahead of the git mirror.
```

Workflow after (copy/paste):

```
A second option is to use svn via the GitHub svn native bridge::

  svn co https://github.com/llvm/llvm-projects/trunk/compiler-rt compiler-rt  —username=...

This checks out only compiler-rt and provides commit access using "svn commit",
in the same way as it would do today.

Finally, you could use *git-svn* and one of the sub-project mirrors::

  # Clone from the single read-only Git repo
  git clone http://llvm.org/git/llvm.git
  cd llvm
  # Configure the SVN remote and initialize the svn metadata
  git svn init https://github.com/joker-eph/llvm-project/trunk/llvm —username=...
  git config svn-remote.svn.fetch :refs/remotes/origin/master
  git svn rebase -l


In this case the repository contains only a single sub-project, and commits can
be made using `git svn dcommit`, again **exactly as we do today**.
```



https://reviews.llvm.org/D24167