[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:204
@@ +203,3 @@
+as we wish and preserve history.
+With the multirepo, moving clang-tools-extra into clang would be
+more complicated than a simple `git mv` command, and we would end up losing
----------------
beanz wrote:
> Google is our friend -> http://stackoverflow.com/questions/1365541/how-to-move-files-from-one-git-repo-to-another-not-a-clone-preserving-history
I don't see `git subtree` at work on this link, just `filter-branch` + `git mv` + merge.

That flow tracks the history of a file, not its **content** AFAIK (i.e. if a function was moved from another file into the current one, the history of when/why this function added/modified won't be included).

Also, what would be the effect of moving a file from a repo to another, and later back to the original repo?

================
Comment at: docs/Proposals/GitHubMove.rst:210
@@ +209,3 @@
+for downstream users that have interest in only a single repository, however
+this is addressed by keeping the single-subproject Git mirrors for each project just as we
+do today. Also the GitHub SVN bridge allows to contribute to a single
----------------
beanz wrote:
> I don't think that our proposals should be constructed as convoluted arguments between contributing authors. Adding pro multi-repo statements will only make this more difficult to grok.
> 
> I actually think there is very little in this section that shouldn't be part of an "arguments/rebuttals" section.
1) There is nothing convoluted here.
2) Adding fact-based pro multi-repo statement will make it easier to understand.
3) I disagree, I think most of this section should stay here. So we'll have to go in the specifics, piece by piece.

================
Comment at: docs/Proposals/GitHubMove.rst:572
@@ +571,3 @@
+
+When the `git bisect run` command returns, the umbrella repository is set to
+the state where the regression is introduced, one can inspect the history on
----------------
beanz wrote:
> You can absolutely *guarantee* the same granularity. You can't guarantee the same ordering, but generally speaking that is significantly less important than granularity.
> 
> To get the same granularity you allow the script that updates submodules to produce more than one commit to the submodule repo at a time. If there are multiple you can sort them by committer date. While committer date isn't a great thing to use since our proposals both depend on maintaining a linear history it should be good enough for the common cases because committer date gets reset on rebase. 
> but generally speaking that is significantly less important than granularity.

No sorry, I can't agree, this is critical: correctness goes before usability. It seems to me that you're willing to trade correctness to bring a guarantee of usability here. 

I'm willing to believe that "in practice" the granularity should be small enough, it just has to be worded carefully. Right now it is a parenthesis at the end:
` (it is possible that one commit in the umbrella repository includes multiple commits in the sub-projects) `, we can reword this  ` (it is possible that one commit in the umbrella repository includes multiple commits in the sub-projects, though it should be occasional in practice)`
(One may bikeshed on what exactly "occasional" is though, but we don't have any data to bikeshed efficiently anyway).

================
Comment at: docs/Proposals/GitHubMove.rst:629
@@ +628,3 @@
+only the projects that are *rev-locked* to LLVM (clang, lld, lldb, ...) and
+leave projects like libcxx and compiler-rt in their own individual and separate
+repositories.
----------------
beanz wrote:
> Ah. I think the confusing phrasing is that monorepo is being used in two contexts. Maybe rephrase this to something like:
> 
> > With this variant of the monorepo proposal developers who only work on excluded sub-projects will continue to use the single-project repositories.
> 
> The workflow is still changed from today, because today we're using SVN.
> 
Sorry, the sentence is really about the monorepo: leaving libcxx within the monorepo should not be a regression compared to today.


https://reviews.llvm.org/D24167