Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Learning the right way to push changes

2010/6/4 Alex Blewitt <alex.blewitt@xxxxxxxxx>
I discovered a bug with the earlier change I'd pushed to the review (and which Mathias kindly committed for me into 2854822da4c717e8df745486b440000639f80d49).

Specifically, I'd forgotten to:
* Dispose of the clipboard
* Handle the case where there is no text (say, because the buffer is empty or because it contains an image)

I also wanted to add:
* Trim the text so that it works if you have leading/trailing spaces

This seems to have resulted in a number of review requests. Ideally, I'd like to do a git commit squash on these three items, and (if no-one has pushed anything on top of the 2854822) squash that as well. But I'm more than happy to leave 2854822 there, and squash the three changes into the 345..1b8 into one commit and push that as a review.

git rebase documentation says :
Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea: anyone downstream of it is forced to manually fix their history.

Hence we only do it if really necessary. Small patches have the advantage that they are easy to review hence their turnaround time
is rather small. If they depend on each other you best do them in multiple commits on one branch and push them in one push to
gerrit (otherwise do them on different topic branches).  Each will end up in a separate change in gerrit but gerrit will recognize that
they are interdependent (e.g. see http://egit.eclipse.org/r/#change,809). Accepted changes in such a patch series are only merged 
by gerrit when all their predecessors have been merged. This also eliminates the need for rebases if they happen to be accepted in
a different order than they have been written. 

In your current case I would say squash the pending changes using rebase -i and push the resulting change.
 
So that I know for next time, what should I have done? Done the commit squash first, and just pushed one thing into the review? If I need to do tweaks in the future, how do I ensure that they are treated as different patch sets on the same review rather than creating new ones?

If you want to replace an existing change already pushed to gerrit you need to add its changeid as a footer in the last paragraph
of the commit message. Gerrit will check if the incoming new change has an already known changeid and if that's the case it
will put it as a new patchset for this change. Otherwise a new one will be created. If you commit using EGit you have to manually
cut & paste the changeid from gerrit (on first push gerrit will generate a changeid if none is found in the commit message). If you
use native git you may install the gerrit changeid hook which will generate the changeid on local commit.
 
In your current case 
806 has no pending predecessors
807 depends on 806
808 depends on 806
809 depends on 808

808 and 807 have the same commit message so you probably intended to replace one of them by the other one
but missed to add the changeid

So the way to go would be
- abandon 807 
- squash the other 3 changes into 806 (means the squashed commit needs its changeid) 
- if necessary tweak this commit again until you are happy (amend is your friend)
- push the squashed commit and check if the result on gerrit is ok
- abandon 808 and 809

If this is confusing let me know, if you want I can also do this for you.

--
Matthias

Back to the top