Working on another branch while a review is ongoing on master
Ash
reviewer is slow so the coder branches to work on another feature in a
topic branch, and does commit B
***A******** A' ******A'' **** F
*******
*
*
**** B **** C *** D
*
A' and A'' are the rebased commits made in response to the review,
which are pushed as patchsets to change1
In the meanwhile though (while waiting on all those review comments),
the coder has been working on a feature in the topic branch, creating
commits B, C and D.
Now A'' and B have diverged significantly as the review suggested a
lot of redesign. So the coder merges his topic branch with master to
create F - which is a merge involving a big conflict resolution to get
all the new features in the topic branch in line with the improved
class structure in A''.
Now he wants to submit F for review as change2.
The problem is that that will automatically push B, C, and D in for
review but we don't want to review them. They contain A-style code -
which has been reworked.
We either:
1) Want to make B, C and D and F patchsets to change2. We would then
ignore B, C and D and review only F.
2) Want to submit only F and pretend B, C, and D never happened.
How do we achieve either of these two options?
We prefer to avoid more conflict resolution (which is difficult as B &
C are very different from A''' whereas F is quite similar as its
already been resolved in the merge)?
I imagine this is a very common scenario but it's not obvious to me
what to do.
Ash
Lok Shenkigul
branches while stuff on other branches is being reviewed - indeed
that's exactly what branches are for!
Gerrit can't really ignore commits as it is merely a git server and
wants to replicate you local repo remotely so forces you to submit
perfect commits by rebasing the old ones.
In your case the commits on the other ("topic") branch must also be
submitted so as you say you can either rebase them and submit them all
as patchsets.
Rebasing past a merge can be a pain though and I don't remember how we
dealt with this when it last occurred - maybe someone else has come
across this? The group does seem awfully quiet - maybe its the
vacation?
yours,
Shenkigul
Ash
rebase everything but I don't know how to upload branch merges as you
have to rebase the merge commit! (If I don't rebase the merge commit,
then the push to gerrit will pull it in as a separate change!)
I'm guessing that people avoid this by rebasing rather than merging
but I'm not clear how that works? Does rebasing merge branches?
How do I get a branch, with it's merges back to another back, all
checked into gerrit as one change?
Magnus Bäck
Ash <ashi...@gmail.com> wrote:
> Say I have commit A that's been pushed for review as change1. The
> reviewer is slow so the coder branches to work on another feature
> in a topic branch, and does commit B
>
> ***A******** A' ******A'' **** F
> *******
> *
> *
> **** B **** C *** D
> *
As later posted on pastebin.com by the OP:
**** A ******* A' **** A'' ******** F ***
* *
* *
* *
*** B **** C **** D ****
> A' and A'' are the rebased commits made in response to the review,
> which are pushed as patchsets to change1
>
> In the meanwhile though (while waiting on all those review comments),
> the coder has been working on a feature in the topic branch, creating
> commits B, C and D.
>
> Now A'' and B have diverged significantly as the review suggested a
> lot of redesign. So the coder merges his topic branch with master to
> create F - which is a merge involving a big conflict resolution to get
> all the new features in the topic branch in line with the improved
> class structure in A''.
You say "merges his topic branch with master", but I assume you really
meant "merges his topic branch with the latest patch set of change 1".
> Now he wants to submit F for review as change2.
>
> The problem is that that will automatically push B, C, and D in for
> review but we don't want to review them. They contain A-style code -
> which has been reworked.
>
> We either:
> 1) Want to make B, C and D and F patchsets to change2. We would then
> ignore B, C and D and review only F.
> 2) Want to submit only F and pretend B, C, and D never happened.
You can't get Git to ignore commits. You could push them past the
review and straight onto the branch so that they wouldn't be subject
to a review, but they'd still be there.
> How do we achieve either of these two options?
>
> We prefer to avoid more conflict resolution (which is difficult as B &
> C are very different from A''' whereas F is quite similar as its
> already been resolved in the merge)?
Don't use merges to update topic branches. What should've been done was
to rebase the second topic branch onto A'' (i.e. the tip of the first
topic branch), possibly squashing B, C, and D into a single commit. This
of course means that if you ever make A''', you'd have to rebase again
(but only resolve any new conflicts -- old ones that were resolved in
the first rebase would be left alone). This is why basing further work
on open changes can be a hassle.
git checkout topic2
git rebase --onto topic1 A
Walk the history backwards until you hit commit A or any ancestor of A,
and copy these commits on top of topic1.
To squash B and C into D you could e.g. use interactive rebase.
--
Magnus Bäck Opinions are my own and do not necessarily
SW Configuration Manager represent the ones of my employer, etc.
Sony Ericsson
Magnus Bäck
Ash <ashi...@gmail.com> wrote:
> Well this narrows down my problem. I'm getting used to having to
> rebase everything but I don't know how to upload branch merges as you
> have to rebase the merge commit! (If I don't rebase the merge commit,
> then the push to gerrit will pull it in as a separate change!)
If you rebase there won't be a merge commit. Rebasing is basically only
an option for topic branches where the commits are private and haven't
published yet since a rebase rewrites each commit and gives them new
SHA-1s.
> I'm guessing that people avoid this by rebasing rather than merging
> but I'm not clear how that works? Does rebasing merge branches?
No, a rebase is fundamentally different from a merge. A rebase gathers a
number of commits and replays them on top of another base. The number of
commits will be the same. If you merge two diverged branches you'll get
an additional commit, the merge commit, that connects the (usually) two
commits that were involved in the merge. This is a bit tricky to explain,
but play around with Git and read the available documentation. The
difference between a rebase and a merge is not specific to Gerrit.
> How do I get a branch, with it's merges back to another back, all
> checked into gerrit as one change?
Sorry, I can't parse this sentence.
Ash
On Aug 1, 10:49 pm, Magnus Bäck <magnus.b...@sonyericsson.com> wrote:
> On Friday, July 29, 2011 at 18:31 CEST,
branch with master/A'' - but we want to push this new commit as a
separate change (i.e. so it becomes change2) so that we can have a
separate discussion about the code that (used to be) in the topic
branch.
>
> > Now he wants to submit F for review as change2.
>
> > The problem is that that will automatically push B, C, and D in for
> > review but we don't want to review them. They contain A-style code -
> > which has been reworked.
>
> > We either:
> > 1) Want to make B, C and D and F patchsets to change2. We would then
> > ignore B, C and D and review only F.
> > 2) Want to submit only F and pretend B, C, and D never happened.
>
> You can't get Git to ignore commits. You could push them past the
> review and straight onto the branch so that they wouldn't be subject
> to a review, but they'd still be there.
>
> > How do we achieve either of these two options?
>
> > We prefer to avoid more conflict resolution (which is difficult as B &
> > C are very different from A''' whereas F is quite similar as its
> > already been resolved in the merge)?
>
> Don't use merges to update topic branches.
workflow allows for people to merge topic branches and push the full
branch history with merge commits to a remote repo. Does gerrit not
like merge commits for some reason?
> What should've been done was
> to rebase the second topic branch onto A'' (i.e. the tip of the first
> topic branch), possibly squashing B, C, and D into a single commit.
> git checkout topic2
> git rebase --onto topic1 A
>
> Walk the history backwards until you hit commit A or any ancestor of A,
> and copy these commits on top of topic1.
>
> To squash B and C into D you could e.g. use interactive rebase.
>
git checkout -b topic
git checkout B; git push refs/for/branch; (B has a new changeId)
git checkout C; git rebase HEAD^; git push refs/for/branch; (this
"deletes" commit C and squashes it into B to create B'')
git checkout D; git rebase HEAD^; git push refs/for/branch; (amends B'
to B'' which contains C & D)
gerrit will then contains change1 which contains all the A commits,
and change2 which contains all the B commits
after submission, the gerrit repo looks like: A'' ******* B'' (i.e.
just two commits) and on the coder's local repo topic branch ==
master?
Say I've rebased C so B is rewritten to B'. Does this mean it's
actually Now the review process demands coder makes A''' (a further
patchset to change1)
Ash
On Aug 1, 11:06 pm, Magnus Bäck <magnus.b...@sonyericsson.com> wrote:
> On Monday, August 01, 2011 at 17:26 CEST,
>
> > Well this narrows down my problem. I'm getting used to having to
> > rebase everything but I don't know how to upload branch merges as you
> > have to rebase the merge commit! (If I don't rebase the merge commit,
> > then the push to gerrit will pull it in as a separate change!)
>
> If you rebase there won't be a merge commit. Rebasing is basically only
> an option for topic branches where the commits are private and haven't
> published yet since a rebase rewrites each commit and gives them new
> SHA-1s.
>
> > I'm guessing that people avoid this by rebasing rather than merging
> > but I'm not clear how that works? Does rebasing merge branches?
>
> No, a rebase is fundamentally different from a merge. A rebase gathers a
> number of commits and replays them on top of another base. The number of
> commits will be the same. If you merge two diverged branches you'll get
> an additional commit, the merge commit, that connects the (usually) two
> commits that were involved in the merge. This is a bit tricky to explain,
> but play around with Git and read the available documentation. The
> difference between a rebase and a merge is not specific to Gerrit.
>
> This
> of course means that if you ever make A''', you'd have to rebase again
> (but only resolve any new conflicts -- old ones that were resolved in
> the first rebase would be left alone). This is why basing further work
> on open changes can be a hassle.
have been squashed together to create B' (the second patchset in
change2) and now we have to make further changes to A, i.e. the A'''
you mentioned.
So as I understand it, since rebase can replay the topic branch's
commits, the history might look like A'' -> B'.
Now we want to create A''' (A triple-prime), we have to checkout A'',
make our commit and rebase to create A'''.
After this, we now have to to rebase B' onto A''' in order to make the
log look like A''' (A triple-prime) -> B', right?
In other words, you've got to continually replay a topic branch's
commits onto a "master" branch in order to keep a linear history which
involves resolving many mini commits instead of one big merge commit.
So my question is, why?
Why can't I just work in topic branch, and create a regular merge
commit "F" and then (before anything has been pushed) rebase these (B,
C, D & F) all together and push these to gerrit (either pushing after
each rebase to create consecutive patchsets to change2 or just push
them as one single squashed commit to create a change2 with one
patchset)?
Why can't I work using merge commits? After all, normal remote git
repos have no problem with merge commits being pushed.
> > How do I get a branch, with it's merges back to another back, all
> > checked into gerrit as one change?
>
> Sorry, I can't parse this sentence.
> > How do I get all the commits in a topic branch, including the merge commit which joined it back to master, all checked into gerrit as one change?
i.e. how do I use branch merges and merge commits instead of replaying
commits onto another branch using rebasing?
Thanks for being so helpful!
Swindells, Thomas
[Swindells, Thomas] You generally shouldn't be developing on an unsubmitted change, there are occasions in a project that you have to but on the whole you shouldn't be, you should be developing off the HEAD of origin/master.
> So my question is, why?
>
> Why can't I just work in topic branch, and create a regular merge commit "F"
> and then (before anything has been pushed) rebase these (B, C, D & F) all
> together and push these to gerrit (either pushing after each rebase to create
> consecutive patchsets to change2 or just push them as one single squashed
> commit to create a change2 with one patchset)?
>
> Why can't I work using merge commits? After all, normal remote git repos
> have no problem with merge commits being pushed.
Because reviewers don't like them as they are really hard to review - how do you do a diff of the change.
While git is distributed Gerrit is effectively a central master branch with HEAD progressing linearly over time and
is optimized and designed to provide a source tree which is linear. The expectation being that test would only take test builds
from a CI machine building HEAD and therefore giving them builds which have a proper ordering of version.
> > > How do I get a branch, with it's merges back to another back, all
> > > checked into gerrit as one change?
> >
> > Sorry, I can't parse this sentence.
>
> I meant:
> > > How do I get all the commits in a topic branch, including the merge
> commit which joined it back to master, all checked into gerrit as one change?
>
> i.e. how do I use branch merges and merge commits instead of replaying
> commits onto another branch using rebasing?
[Swindells, Thomas] Why are you so intent on doing so? The end result is the same giving you A+B, the difference is rebasing provides a much cleaner history of what was changed when on the official release codeline. Most people take the view intermediate states produced by developers as they are producing their final commit are irrelevant and unneeded to be preserved, it's just the history of the order that changes were pushed onto the codeline that matters.
> Thanks for being so helpful!
>
> >
> > --
> > Magnus Bäck Opinions are my own and do not
> > necessarily SW Configuration Manager represent the ones of my
> employer, etc.
> > Sony Ericsson
>
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
**************************************************************************************
This message is confidential and intended only for the addressee. If you have received this message in error, please immediately notify the postm...@nds.com and delete it from your system as well as any copies. The content of e-mails as well as traffic data may be monitored by NDS for employment and security purposes. To protect the environment please do not print this e-mail unless necessary.
NDS Limited. Registered Office: One London Road, Staines, Middlesex, TW18 4EX, United Kingdom. A company registered in England and Wales. Registered no. 3080780. VAT no. GB 603 8808 40-00
**************************************************************************************
Ash
review can be very slow, taking days between comments and revisions/
patchsets. In the meanwhile, the coder wants to press on developing
future features and not twiddling their thumbs, bored.
>
> > So my question is, why?
>
> > Why can't I just work in topic branch, and create a regular merge commit "F"
> > and then (before anything has been pushed) rebase these (B, C, D & F) all
> > together and push these to gerrit (either pushing after each rebase to create
> > consecutive patchsets to change2 or just push them as one single squashed
> > commit to create a change2 with one patchset)?
>
> > Why can't I work using merge commits? After all, normal remote git repos
> > have no problem with merge commits being pushed.
>
> Because reviewers don't like them as they are really hard to review - how do you do a diff of the change.
> While git is distributed Gerrit is effectively a central master branch with HEAD progressing linearly over time and
> is optimized and designed to provide a source tree which is linear. The expectation being that test would only take test builds
> from a CI machine building HEAD and therefore giving them builds which have a proper ordering of version.
>
> > > > How do I get a branch, with it's merges back to another back, all
> > > > checked into gerrit as one change?
>
> > > Sorry, I can't parse this sentence.
>
> > I meant:
> > > > How do I get all the commits in a topic branch, including the merge
> > commit which joined it back to master, all checked into gerrit as one change?
>
> > i.e. how do I use branch merges and merge commits instead of replaying
> > commits onto another branch using rebasing?
>
> [Swindells, Thomas] Why are you so intent on doing so? The end result is the same giving you A+B, the difference is rebasing provides a much cleaner history of what was changed when on the official release codeline. Most people take the view intermediate states produced by developers as they are producing their final commit are irrelevant and unneeded to be preserved, it's just the history of the order that changes were pushed onto the codeline that matters.
to review however that involves a lot of rebasing and resolving
conflicts if two things are going on at the same time.
In the case of a small team (my situation) I don't mind seeing
branching and merging especially if all the branches and merges are
included as consecutive patchsets within one change - which is what I
was originally asking last Friday. I'd rather have to view unsightly
branching+merging than forcing my developer to waste time continually
rebasing and resolving conflicts when they're not even ready to submit
for review!
As I said above, in the case of slow reviews, it is very common that
you want to press ahead with working on future features while the
review goes on at its own pace. Essentially, you probably don't
encounter this problem as I'm guessing you work in an environment
where there are many reviewers and reviews are done promptly and so
everyone can take a few hours off and concentrate on the review so
there's no issues in having to work on other work in the meanwhile.
Surely the option to submit merge conflict commits should be mine and
I'm sure there is a way to allow them to be included, even with the
need to squash those merge commits via rebase.
>
> > Thanks for being so helpful!
>
> > > --
> > > Magnus Bäck Opinions are my own and do not
> > > necessarily SW Configuration Manager represent the ones of my
> > employer, etc.
> > > Sony Ericsson
>
> > --
> > To unsubscribe, email repo-discuss...@googlegroups.com
> > More info athttp://groups.google.com/group/repo-discuss?hl=en
>
> *************************************************************************** ***********
Swindells, Thomas
If they have changes A->B->C
And need to fix A then what they should do is an interactive rebase, fixup A, add the new changes and then resume the rebase, this will then rebase B->C on top of A'
It's when you are developing off other peoples pending changes that things get messier.
Thomas
**************************************************************************************
This message is confidential and intended only for the addressee. If you have received this message in error, please immediately notify the postm...@nds.com and delete it from your system as well as any copies. The content of e-mails as well as traffic data may be monitored by NDS for employment and security purposes. To protect the environment please do not print this e-mail unless necessary.
Magnus Bäck
Ash <ashi...@gmail.com> wrote:
Right. To me you're looking for a rebase of F onto A''.
> > > Now he wants to submit F for review as change2.
> >
> > > The problem is that that will automatically push B, C, and D in for
> > > review but we don't want to review them. They contain A-style code -
> > > which has been reworked.
> >
> > > We either:
> > > 1) Want to make B, C and D and F patchsets to change2. We would then
> > > ignore B, C and D and review only F.
> > > 2) Want to submit only F and pretend B, C, and D never happened.
> >
> > You can't get Git to ignore commits. You could push them past the
> > review and straight onto the branch so that they wouldn't be subject
> > to a review, but they'd still be there.
> >
> > > How do we achieve either of these two options?
> >
> > > We prefer to avoid more conflict resolution (which is difficult as
> > > B & C are very different from A''' whereas F is quite similar as
> > > its already been resolved in the merge)?
> >
> > Don't use merges to update topic branches.
>
> Out of interest, why not? A typical git (not gerrit) distributed
> workflow allows for people to merge topic branches and push the full
> branch history with merge commits to a remote repo. Does gerrit not
> like merge commits for some reason?
Gerrit doesn't mind merge commits as such, although at least until
recently the support for reviewing merge commits in the web UI has
been quite limited. However, merge commits that are pushed to
refs/for/foo will still end up as changes. Why would you want to
spend time on reviewing them? Gerrit is tuned for a workflow where
a topic branch is like the personal workspace of a single user. It
isn't shared with anyone, so rebases are okay (and expected), and
using merges to update the branch would just result in useless merge
commits.
> > What should've been done was to rebase the second topic branch onto
> > A'' (i.e. the tip of the first topic branch), possibly squashing B,
> > C, and D into a single commit. <cut>
> >
> > git checkout topic2
> > git rebase --onto topic1 A
> >
> > Walk the history backwards until you hit commit A or any ancestor
> > of A, and copy these commits on top of topic1.
> >
> > To squash B and C into D you could e.g. use interactive rebase.
>
> OK thanks, so to be clear what should have been done in my example is:
> git checkout -b topic
Why? You're immediately switching to another commit and you're not using
this branch for anything in the rest of your example.
> git checkout B; git push refs/for/branch; (B has a new changeId)
Um, okay, but why push the B commit to Gerrit if you intend to squash
B, C, and D?
> git checkout C; git rebase HEAD^; git push refs/for/branch; (this
> "deletes" commit C and squashes it into B to create B'')
No, "git rebase HEAD^" is a no-op. It means "rebase the current commit
onto the first parent of the current commit".
> git checkout D; git rebase HEAD^; git push refs/for/branch; (amends B'
> to B'' which contains C & D)
>
> gerrit will then contains change1 which contains all the A commits,
> and change2 which contains all the B commits
> after submission, the gerrit repo looks like: A'' ******* B'' (i.e.
> just two commits) and on the coder's local repo topic branch ==
> master?
You've got the idea right, but your method if flawed. Either of the
following methods work though. Apart from the differences in the commit
messages (if you don't alter them yourself), about the only difference
I can think of is that the first example will leave the topic branch
pointing to commit D rather than being rebased to the squashed commit
that's on top of A''
git checkout A''
git merge --squash topic
git commit
git checkout topic
git rebase -i --onto A'' A
<change "pick" to "squash" for all but the first commit>
> Say I've rebased C so B is rewritten to B'. Does this mean it's
> actually Now the review process demands coder makes A''' (a further
> patchset to change1)
Sorry, this paragraph doesn't make sense to me.
Magnus Bäck
Ash <ashi...@gmail.com> wrote:
> So let's pretend we're at a point where A'' has been pushed, and B & C
> have been squashed together to create B' (the second patchset in
> change2) and now we have to make further changes to A, i.e. the A'''
> you mentioned.
>
> So as I understand it, since rebase can replay the topic branch's
> commits, the history might look like A'' -> B'.
>
> Now we want to create A''' (A triple-prime), we have to checkout A'',
> make our commit and rebase to create A'''.
You can probably use rebase for this procedure, but a regular "commit
--amend" would be the typical method to further enhance the A commit.
> After this, we now have to to rebase B' onto A''' in order to make the
> log look like A''' (A triple-prime) -> B', right?
Yes.
> In other words, you've got to continually replay a topic branch's
> commits onto a "master" branch in order to keep a linear history which
> involves resolving many mini commits instead of one big merge commit.
>
> So my question is, why?
You don't have to do the rebases all the time, you could just do a
single rebase at the end when the B change is ready to be submitted.
That would make this workflow involve just as many rebase/merge
operations as if you'd use a merge-based workflow that you seem to
favor.
Let's look at this from a different perspective. What if you need to
update your topic branch (with the B commit) from both A' and A''? If
you later on upload A''' and submit that to the branch your Git commit
graph would look horrible:
----------B
/ / /
---A /
/ /
------A'
/
X-------------A'''
^
master
Your B commit (or B' or B'' or whatever the number of patch sets you'd
have) would contain both the A and A' commits (because you've merged
from them), yet none of those commits were approved and have never been
present on the master branch. That doesn't make sense!
> Why can't I just work in topic branch, and create a regular merge
> commit "F" and then (before anything has been pushed) rebase these
> (B, C, D & F) all together and push these to gerrit (either pushing
> after each rebase to create consecutive patchsets to change2 or just
> push them as one single squashed commit to create a change2 with one
> patchset)?
I'm sorry, but I don't understand what it is that you want to do and why
it would be superior to Gerrit's standard workflow. Maybe you're using
terminology in an unexpected way, or maybe I'm just losing my patience.
Why create the F merge commit if you're rebasing B, C, and D anyway?
Why do you believe the approach of squashing the local commits into a
single commit and uploading that one contradicts the standard workflow
that I'm promoting?
[...]
Magnus Bäck
Ash <ashi...@gmail.com> wrote:
> It's cleaner to rebase and get one long linear codeline, and simpler
> to review however that involves a lot of rebasing and resolving
> conflicts if two things are going on at the same time.
I don't agree.
> In the case of a small team (my situation) I don't mind seeing
> branching and merging especially if all the branches and merges are
> included as consecutive patchsets within one change - which is what I
> was originally asking last Friday. I'd rather have to view unsightly
> branching+merging than forcing my developer to waste time continually
> rebasing and resolving conflicts when they're not even ready to submit
> for review!
If they're not ready to upload their changes for review, they don't have
to rebase their code. At all. I don't know why you think there's a need
for continuous rebases. Such a need will only arise if you have two
consecutive commits that have been uploaded are being reviewed at the
same time. If commit A is updated to A' you *will* have to update B to
B' *if you want to submit the B change*. This is no different from the
case of using merge commits -- while you could leave the B change alone
you'd have to update (redo) the merge commit instead. This doesn't
strike me as easier. If A involves multiple patch sets and B doesn't,
this is what it could look like, if we picture the flow with a
downward-facing timeline and one column for each change involved:
make commit A
hack
make commit B
make commit A'
hack
make commit A''
approval
submit to branch
rebase from A''
approval
submit to branch
If you instead want to use merges it would (I guess) look like this:
make commit A
hack
make commit B
make commit A'
hack
make commit A''
approval
submit to branch
approval
merge B and A'' to form C
approval
submit to branch
submit to branch
To me the conflict resolution effort involved is identical, but the
second way involves an additional (useless) change and is generally
more complicated.
> As I said above, in the case of slow reviews, it is very common that
> you want to press ahead with working on future features while the
> review goes on at its own pace. Essentially, you probably don't
> encounter this problem as I'm guessing you work in an environment
> where there are many reviewers and reviews are done promptly and so
> everyone can take a few hours off and concentrate on the review so
> there's no issues in having to work on other work in the meanwhile.
No, that's not the case. We have a lot of parallel work while review for
past commits is still ongoing. The reason we don't see the problems
you're seeing is that we think rebases are a fine way of keeping topic
branches up to date. I still haven't understood why you favor merges.
> Surely the option to submit merge conflict commits should be mine
> and I'm sure there is a way to allow them to be included, even with
> the need to squash those merge commits via rebase.
Sorry, not getting this.