clattner at apple.com
Fri Jul 29 11:01:20 PDT 2011
On Jul 28, 2011, at 2:16 PM, David A. Greene wrote:
>>> The flow promoted by Git is precisely to make sure each and every commit
>>> passes the tests. So, the granularity of "incremental development" is
>>> really the commit, not how often you merge.
>> This model is based on the idea of some trusted maintainer doing code
>> review of the branch and then pushing it to mainline as a huge batch
>> when everything is happy and working. This is fundamentally in
>> tension with the LLVM model and I have no desire for us to switch.
> Ah. Here is where I think we have some miscommunication happening.
> Here's how I am thinking about things. Reviews should keep happening
> the way they are.
> In fact, what we are doing now is exactly the same
> work one would have to do to review a git branch, but that's immaterial.
Disagreed. The point is that I should not see a stream of 20 decomposed patches from you. When I get to one that is "wrong" or needs changes (e.g. patch 6), then all the other patches after it get ignored. This is silly. Instead, what I should see are (for example) and be asked to review them. Here's a hypothetical example of what I want:
Send in or directly commit 5 patches if they are "obvious" cleanups. These are patches that just move things around in obvious ways, fix formatting, whatever.
Send in a patch 6 and wait for review of *just it* saying "this does something crazy, here is why, here is why it is the best thing to do". Someone will review it, and if there are any problems, it goes back to you for revision. It eventually goes in when it is good for the tree.
After that patch is in, another stream of obvious patches are unblocked and go directly in.
Patch 12 comes around, gets reviewed just like #6... etc.
I *don't* want to see all 20 patches up front.
> I think the disagreement (if we can call it that) is really over what
> actually gets pushed to "mainline" which I assume would be defined as
> "master" on llvm.org.
No. The fundamental disagreement/misunderstanding here is that you are optimizing for an out-of-tree maintainer pushing "batches" of work upstream. This is not what I'm at all interesting in optimizing for, and if you're irritated about the turn-around time that it takes to get review, then you shouldn't be optimizing for this either.
More information about the llvm-dev