[PATCH] D24167: Moving to GitHub - Unified Proposal
Renato Golin via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 1 18:07:49 PDT 2016
rengolin added a subscriber: rengolin.
rengolin added a comment.
Hi Medhi,
Thanks for the proposal, it's looking really great. You did a great job at the workflows, explaining each with commands and difficulties, and I think that's going to help a lot people decide one way or another.
But by being mixed together, I agree with Chris that it looks muddled a bit, and we may need a more impartial description of both proposals, and keeping them separate is one way of doing it.
Another would be to make sure you only use neutral phrases, make sure both proposals are covered for all topics, even if redundant. But that could lead, as Chris said well, to passionate backlash from you "changing their texts" even if it was completely unintentional or misinterpreted.
So, to avoid another round of arguments, it'd probably be best to just describe the monorepo, update the old description with the additional workflow. You already have all the material, so it should be a matter of copy & pasting the current text inside the other, *adding* content, so that people can "relax" that their proposal is not being "muddled". :)
Also, a very clear way to clearly lay out the facts is to build a table at the end with the summary and back-links to the text, where relevant.
Example:
Topic SVN Mono Multi
Clone/Build one svn co clone+sparse clone
Clone/Build all svn co... clone clone+submodules
Bisect scripts natural umbrella
...
I have a number of comments that are still relevant if you merge this into the old doc. Few questions about the technical side, some typos / nits, and some misunderstanding from my part as to what you mean (others may see it that way, too, so...).
But overall, it's looking really good, thank you for all the effort!
cheers,
--renato
================
Comment at: docs/Proposals/GitHubMove.rst:148
@@ +147,3 @@
+..
+ TODO: Is this going to work when people push via the SVN bridge?
+
----------------
This is a good question. If it works at all, two things can happen:
1. SVN reports rev 123, I commit, get rev 124. Git rev-count get's 124
2. SVN reports rev 123, I commit, get rev 125 because someone committed at the same time and git sorted the other commit first.
If 2 happens, I don't think it'll be a big deal, so, we should be fine, as long as the SVN bridge can work with the linearity enforcement of restricted branches.
================
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
----------------
No need, GitHub has email hooks: https://help.github.com/articles/managing-notifications-for-pushes-to-a-repository/
================
Comment at: docs/Proposals/GitHubMove.rst:183
@@ +182,3 @@
+http://llvm.org/git/compiler-rt.git) with git-svn read-write access would be
+maintained
+
----------------
Full stop.
================
Comment at: docs/Proposals/GitHubMove.rst:199
@@ +198,3 @@
+as we wish. With the multirepo, moving clang-tools-extra into clang would be
+much more complicated, and might end up loosing history.
+
----------------
Better to avoid "much more" for the reasons we have discussed before. Either say how it's worse, or don't compare.
================
Comment at: docs/Proposals/GitHubMove.rst:211
@@ +210,3 @@
+ensure that it's easy to set up your build to compile only a few particular
+sub-projects.
+
----------------
I think this won't address the fears of people that don't know enough to not panic. This is why getting the technical parts correct and accurate is so important (and I confess I didn't do enough due diligence on my part of the text either).
================
Comment at: docs/Proposals/GitHubMove.rst:225
@@ +224,3 @@
+Under the multirepo, things are more involved. We describe here the proposed
+solution.
+
----------------
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.
================
Comment at: docs/Proposals/GitHubMove.rst:249
@@ +248,3 @@
+One example of such a repository is Takumi's llvm-project-submodule
+(https://github.com/chapuni/llvm-project-submodule). You can use
+`git submodule init` to check out only the sub-projects you're interested in, and
----------------
Can you add a link of the monorepo as well? I think you had one, right?
================
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
+
----------------
Better to keep the same order svn/git. And don't need to specify that it's a bridge, since you mention above.
================
Comment at: docs/Proposals/GitHubMove.rst:296
@@ +295,3 @@
+
+Commits are performed using `svn commit` or `git commit` and `git svn dcommit`.
+
----------------
Can you commit via git directly?
================
Comment at: docs/Proposals/GitHubMove.rst:322
@@ +321,3 @@
+You can commit as normal using `git commit` and `git push` or `svn commit`, and
+read the history for a single project (`git log libcxx` for example).
+
----------------
Is this a flat tree (like today) or the checked-out tree (tools/clang, etc)?
================
Comment at: docs/Proposals/GitHubMove.rst:349
@@ +348,3 @@
+change that prevented the push. An immediate repeat of the command would
+(almost) certainly result in a successed push. (This is
+an extra step that you don't need in the multirepo, but for those of us who
----------------
successful?
================
Comment at: docs/Proposals/GitHubMove.rst:423
@@ +422,3 @@
+ git submodule init
+ git submodule update clang llvm libcxx
+
----------------
Maybe mention --recursive, too?
================
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.
+
----------------
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.
================
Comment at: docs/Proposals/GitHubMove.rst:584
@@ +583,3 @@
+
+As a demonstration, we've migrated the "Cherry" fork to the monorepo in two ways:
+
----------------
"CHERI"
================
Comment at: docs/Proposals/GitHubMove.rst:599
@@ +598,3 @@
+always possible: you can apply the patches in the appropriate subdirectory of
+the monorepo.
+
----------------
... or you can apply directly on the multi-repo solution.
It's good to repeat to make clear that both solutions are covered.
================
Comment at: docs/Proposals/GitHubMove.rst:607
@@ +606,3 @@
+leave projects like libcxx and compiler-rt in their own individual and separate
+repository.
+
----------------
I'd add a small paragraph explaining the problems that will come from having "two worlds", neither here, nor there. If it's too complicated, than lets not even propose that, as it'll end up as a third proposal.
================
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.)
+
----------------
Can they checkout the read-only libc++ and commit without checking out the entire monorepo?
================
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/
----------------
This is confusing... I thought you were going to list all of them, mono and multi.
https://reviews.llvm.org/D24167
More information about the llvm-commits
mailing list