[Development] Pushing rebases and unrelated changes together is a sin

Shawn Rutledge shawn.t.rutledge at gmail.com
Wed Feb 20 13:01:46 CET 2013


On 20 February 2013 09:43, Samuel Rødal <samuel.rodal at digia.com> wrote:
> On 02/20/2013 09:22 AM, Thiago Macieira wrote:
>> On quarta-feira, 20 de fevereiro de 2013 08.47.40, Samuel Rødal wrote:
>>> The correct process when doing rebases is:
>>>
>>> git pull --rebase # or your favorite git work-flow
>>> git push gerrit HEAD:refs/for/some_branch
>>> # add comment in change on gerrit about being a pure rebase
>>> # do some changes
>>> git commit -a --amend
>>> git push gerrit HEAD:refs/for/some_branch
>>
>> Actually, I'll go further: the above should be done ONLY if you are trying to
>> solve a conflict. If you're not trying to solve a conflict, DON'T REBASE.
>
> I thought that went without saying :)

When revisiting a patch from a few days or more ago, I guess I just
don't trust that the change will apply cleanly or is definitely still
applicable without further changes.  So that would be the reason to
cherry-pick and ensure it, rather than assuming it will be OK and then
waiting for a CI failure, right?  Fine, if the rebase has to be a
separate push, we can just clutter up gerrit with more pushes.  I
typically do plenty of them anyway, due to various mistakes, to sync
up between machines, to store intermediate work and so on.  (And then
people make snide comments like "geez, do they pay you by the
patch?!?" so somebody is always dissatisfied.)

Coming back and asking "would you please split the rebase from the
changes" is also too easy for the reviewer to ask for, and not so easy
to actually do, after having mixed them up.

Then there's the scenario when my change depends on someone else's:
then I don't have a choice - I have to base my work on top of the
latest checkout (NOT latest cherry-pick) of his patch, in order to
avoid doing a rebase of that change (plus its dependencies) when
pushing.  Then all of the patches get out-of-date together. And then
some reviewer gives a -1 because I've been working on a checkout
rather than a cherry-pick, so s/he is the one who finds out that it no
longer applies cleanly.  Now suppose the patch(es) that I depend on
are rebased, and I didn't realize it: next time I push, I will push an
outdated version of his patch(es) along with mine.  Maybe that should
be more interactive too, somehow, to be warned about such mistakes.
Or maybe I should come up with a better way to push a single, isolated
patch, without its dependencies.  But then the trouble would be that
you won't see the dependencies in gerrit, and have to remember the
right order to merge them.

At the very least, gerrit needs an undo function, or a way to make a
previous version of a patch become the latest for review again.

And I agree it would be nice if gerrit could still show a clean diff
even when a patch contains both a rebase and some actual changes.

So this bookkeeping stuff is still more complicated than it ought to
be, IMO.  I would very much like to be able to just concentrate on the
code, but actually 90% of my job is overhead of various kinds.  I'm
not exaggerating.



More information about the Development mailing list