So, I do don't like history that look like the following:
> commit eeee: last nits from reviewer> commit dddd: oops, typo that prevented
> compilation> commit cccc: some more fix found during review> commit bbbb:
> refactor half of preceding patch following reviewer comments> commit aaaa: Do
> something awesome - patch for #666
To be sure we talk of the same thing, let it be clear that I'm talking
aboutmultiple commits due to the review process, i.e. changeset that
is separatedinto multiple commits only because that is the history of
the mistakes madeby the author and remarked by the reviewer (or even
that the author seeafter having made it's branch public).
Now I could sum the reason why I think this is a regression compared
to therather clean history we've had so can be summed up by two
things:
1) I cannot see any case where having those details once the ticket
iscommitted to the project (i.e, has been +1'ed) would be useful, and
Icertainly never ever wanted to know those when looking at our
history.
2) I do think that keeping those details would be counter-productive:
- It makes reading the history harder by the sheer fact of the added
volume of commits. And since those commits are noise once the feature
is committed (again, when knowing that the initial author had
forgot to check the tests were not compiling and had to commit
another one liner, or had tried another approach that has been
heavily refactored in the next patch because the first attempt was
ugly has ever be useful?). - It potentially makes bisect harder.
Again, I'm talking about commits coming from the review process.
Those, more or less by definition, come to fix mistakes made
initially. They will more often than not not be compiling, or maybe
just have the tests that do not compile, or introduce bugs that are
fixed in the very next commit. When you bisect, those kind of
commit are a pain in the ass. - It makes it more annoying to find
what commits refer to what ticket. Not impossible, just a little
harder.
Let it be clear that I'm not saying keeping the 'review commits'
makeanything *impossible*, but it does make very common tasks more
complicated(and potentially very frustrating in the case of bisect),
while adding onlynoise.
> The other side of the equation is that there is tremendous power to be had>
> from distributed versioning, and this proposed workflow discourages taking>
> advantage of that
Would you mind elaborating, maybe be a little more concrete on that
one.Because just to be such we agree, I did not propose to rebase
publicbranches or something like that.
I proposed basically that once a changeset has been finalized on a
branchxxx of the author repo, instead of merging the branch xxx, the
committerwould (locally) extract the content of that changeset and
commit is a onecommit to the project repository (say trunk). Given
that git is contentbased, it "should" work without much problem for
other branch based on xxxto merge trunk in their branch.
Sure that may not be "beautiful" or something, and I you have better
topropose, be my guest.
Last thing, I did check the kernel source git history (which as far as
Iknow is considered as a model of git usage and distribution), and
there isnot a single commit that looks like some nits issue from the
review of apreceding commit.
--Sylvain
