[LLVMdev] git

[Óscar Fuentes]

Chris Lattner <clattner at apple.com> writes:

[snip]

> 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.

This is not against git way of doing things. Actually, using git would
easy the process for the submitter, as it provides convenient methods
for decomposing and combining source code changes. And the reviewer
would keep working as he does now.

>> 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.

>From my understanding of David's position, there is no disagreement here
either. As mentioned above, there is no problem with git for pushing a
series of non-controversial changes and then submit for review some
others. However, what David (and me) wish to point out is that sometimes
presenting a series of patches for review may be also convenient for the
reviewer, because he then can say "patch #1 looks fine but it is not
really required by what you do in patch #2". Overall, the number of
patches submitted for review would be the same as today (letting aside
any effect git may induce in the productivity of some developers) but
instead of seeing 2 patches from Alice and 2 from Bob today you would
see 4 from Alice today and 4 from Bob tomorrow. There are other
advantages, like keeping a more cohesive history of changes, and
avoiding up to some point erratic development (people who commit
preparatory changes too early, and then commit corrections for the
previous changes...) but things like that you will observe with practice
and always can adapt the micro-polices accordingly.

Really, there are no disagreements on the review process, just
misunderstandings.