Challenges with Rebase If Necessary strategy

[David Erickson]

Hi All-
Our team has been using gerrit for a couple of months now with the rebase if necessary strategy.  This strategy is alluring from the standpoint of achieving a very clean and straight repo history, and all of us individually preferred git development where we pull --rebased all our patches.  However in a team setting we've run into a number of issues that are quickly making Gerrit the bottleneck in our coding process for reviewers, and I'm wondering how others have solved these, or how other strategies solve these challenges.  Most of the challenges seem to revolve around dependencies, where on our team our developers tend to send in a small number of patches that have dependencies on each other.

-The first challenge from the UI is how to determine the the 'oldest' change, or the one with the fewest dependencies.  Neither the main Gerrit My Reviews UI, nor the total list of open changes appear to have any way to achieve this type of sort.  I have settled for picking a change, and then looking at its "Related Changes" list and clicking down through them.  This works, but is time consuming, and eventually near the bottom of the list clicking takes you to gitweb instead of a change, and you know at that point it is merged, and to start from the prior change.

-Once the change is located that has its dependencies already merged, the next problem that frequently occurs is that I hit Code-Review +2 and find that I can't merge it because its parent is actually an older patchset.  At that point I have to click rebase, then Code-Review+2 again, wait for it to be verified by Jenkins, and then click submit again.

-Related to the prior challenge, once any change has to be rebased, all of its children will then be forced to be rebased one by one to ultimately be merged... I think you can see the pain here.

And more recently we've had a number of smaller commits coming in at a much higher frequency, and the thought of having to walk every single one of them through the above process is very painful.  Is this happening to everyone else as well?

If I could describe my ideal workflow, it would like something like this:

-The UI makes it obvious which changes I should start from based on their 'mergability', maybe via indentation or otherwise
-I can reasonably quickly go through the list, scoring commits as needed, either +2 or -1 along with feedback.
-Changes ready to be submitted I clicked Code Review +2, and submit.
-If a change has all its dependencies merged, it merges right away.
-If a change has all its dependencies merged, but its parent is out of date, it auto-rebases on the newer patchset, if that succeeds then it copies my approval to the new patchset, re-runs Jenkins, and if that succeeds it auto merges (hands off).
-The above two steps repeat recursively checking to see if any child changes that were waiting on the merged change can now be merged themselves (possibly with a rebase).

For me the high level bit is, if I've code reviewed a commit and told it to submit, I don't want to have to go back to that change and take any more action, unless a new patchset is sent in by a committer, or there is some kind of merge conflict.

Thoughts?

Thanks,
David

[Dave Borowitz]

On Mon, Feb 3, 2014 at 9:25 AM, David Erickson <halcy...@gmail.com> wrote:

Hi All-
Our team has been using gerrit for a couple of months now with the rebase if necessary strategy.  This strategy is alluring from the standpoint of achieving a very clean and straight repo history, and all of us individually preferred git development where we pull --rebased all our patches.  However in a team setting we've run into a number of issues that are quickly making Gerrit the bottleneck in our coding process for reviewers, and I'm wondering how others have solved these, or how other strategies solve these challenges.  Most of the challenges seem to revolve around dependencies, where on our team our developers tend to send in a small number of patches that have dependencies on each other.

-The first challenge from the UI is how to determine the the 'oldest' change, or the one with the fewest dependencies.  Neither the main Gerrit My Reviews UI, nor the total list of open changes appear to have any way to achieve this type of sort.  I have settled for picking a change, and then looking at its "Related Changes" list and clicking down through them.  This works, but is time consuming, and eventually near the bottom of the list clicking takes you to gitweb instead of a change, and you know at that point it is merged, and to start from the prior change.

-Once the change is located that has its dependencies already merged, the next problem that frequently occurs is that I hit Code-Review +2 and find that I can't merge it because its parent is actually an older patchset.  At that point I have to click rebase, then Code-Review+2 again, wait for it to be verified by Jenkins, and then click submit again.

-Related to the prior challenge, once any change has to be rebased, all of its children will then be forced to be rebased one by one to ultimately be merged... I think you can see the pain here.

And more recently we've had a number of smaller commits coming in at a much higher frequency, and the thought of having to walk every single one of them through the above process is very painful.  Is this happening to everyone else as well?

If I could describe my ideal workflow, it would like something like this:

-The UI makes it obvious which changes I should start from based on their 'mergability', maybe via indentation or otherwise

2.9 will have a column for this in the change table, as well as an is:mergeable operator.

 

-I can reasonably quickly go through the list, scoring commits as needed, either +2 or -1 along with feedback.

Is the idea that you want to do this for the entire list of changes, independent of their relationship? The typical workflow allows this kind of review by navigating through related changes.

 

-Changes ready to be submitted I clicked Code Review +2, and submit.
-If a change has all its dependencies merged, it merges right away.
-If a change has all its dependencies merged, but its parent is out of date, it auto-rebases on the newer patchset, if that succeeds then it copies my approval to the new patchset,

I think it would be reasonable for Rebase If Necessary changes to be submitted even if not all parents are submitted. This isn't the way Gerrit works now, however.

 

re-runs Jenkins, and if that succeeds it auto merges (hands off).

There is no support for auto-submitting a change as soon as it becomes submittable. But you're already using Jenkins, you can just tell Jenkins to submit it, right?

 

-The above two steps repeat recursively checking to see if any child changes that were waiting on the merged change can now be merged themselves (possibly with a rebase).

Submitting a sequence of changes kind of works like this right now. If you have three submittable changes A<-B<-C, you can submit them in the order C, B, A, and they will submit atomically as soon as you click submit for A.

 

For me the high level bit is, if I've code reviewed a commit and told it to submit, I don't want to have to go back to that change and take any more action, unless a new patchset is sent in by a committer, or there is some kind of merge conflict.

As I said above, I think this is not impossible. We would need to have the merge queue code treat "submittable and rebase-if-necessary" just like it does "submitted but not merged", i.e. consider all such changes when processing a merge on the branch.

 

Thoughts?

Thanks,
David

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
 
---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[David Erickson]

Hi Dave-
Thanks for your response, see inline:

On Monday, February 3, 2014 2:16:25 PM UTC-8, Dave Borowitz wrote:

On Mon, Feb 3, 2014 at 9:25 AM, David Erickson <halcy...@gmail.com> wrote:

Hi All-
Our team has been using gerrit for a couple of months now with the rebase if necessary strategy.  This strategy is alluring from the standpoint of achieving a very clean and straight repo history, and all of us individually preferred git development where we pull --rebased all our patches.  However in a team setting we've run into a number of issues that are quickly making Gerrit the bottleneck in our coding process for reviewers, and I'm wondering how others have solved these, or how other strategies solve these challenges.  Most of the challenges seem to revolve around dependencies, where on our team our developers tend to send in a small number of patches that have dependencies on each other.

-The first challenge from the UI is how to determine the the 'oldest' change, or the one with the fewest dependencies.  Neither the main Gerrit My Reviews UI, nor the total list of open changes appear to have any way to achieve this type of sort.  I have settled for picking a change, and then looking at its "Related Changes" list and clicking down through them.  This works, but is time consuming, and eventually near the bottom of the list clicking takes you to gitweb instead of a change, and you know at that point it is merged, and to start from the prior change.

-Once the change is located that has its dependencies already merged, the next problem that frequently occurs is that I hit Code-Review +2 and find that I can't merge it because its parent is actually an older patchset.  At that point I have to click rebase, then Code-Review+2 again, wait for it to be verified by Jenkins, and then click submit again.

-Related to the prior challenge, once any change has to be rebased, all of its children will then be forced to be rebased one by one to ultimately be merged... I think you can see the pain here.

And more recently we've had a number of smaller commits coming in at a much higher frequency, and the thought of having to walk every single one of them through the above process is very painful.  Is this happening to everyone else as well?

If I could describe my ideal workflow, it would like something like this:

-The UI makes it obvious which changes I should start from based on their 'mergability', maybe via indentation or otherwise

2.9 will have a column for this in the change table, as well as an is:mergeable operator.

Great, anything to make it obvious what the dependency hierarchy is at a glance from the main UI list would be really helpful.
 

 

-I can reasonably quickly go through the list, scoring commits as needed, either +2 or -1 along with feedback.

Is the idea that you want to do this for the entire list of changes, independent of their relationship? The typical workflow allows this kind of review by navigating through related changes.

I think you are probably right that related changes will get priority and iterated through in order there, then moving on to other changes.

 

 

-Changes ready to be submitted I clicked Code Review +2, and submit.
-If a change has all its dependencies merged, it merges right away.
-If a change has all its dependencies merged, but its parent is out of date, it auto-rebases on the newer patchset, if that succeeds then it copies my approval to the new patchset,

I think it would be reasonable for Rebase If Necessary changes to be submitted even if not all parents are submitted. This isn't the way Gerrit works now, however.

I think you can do this today as far as submitting goes, and it will sit in queue saying 'merge pending', and give errors in the messages on the actual change.  The problem though is that once the parents are satisfied, you have to circle back, click rebase, wait for tests, then manually submit and merge again.
 

 

re-runs Jenkins, and if that succeeds it auto merges (hands off).

There is no support for auto-submitting a change as soon as it becomes submittable. But you're already using Jenkins, you can just tell Jenkins to submit it, right?

This might be doable, as long as Jenkins cannot submit it in the absense of a +2 code review (which I think is currently configured).  Were this working, I thing the remaining needed items would be:
-Keep any existing code review scores when "Rebase" is clicked
-After submission, iterate through any commits depending on the merged commit to see if the process can automatically proceed forward for them
 

 

-The above two steps repeat recursively checking to see if any child changes that were waiting on the merged change can now be merged themselves (possibly with a rebase).

Submitting a sequence of changes kind of works like this right now. If you have three submittable changes A<-B<-C, you can submit them in the order C, B, A, and they will submit atomically as soon as you click submit for A.

Assuming they are all based on current versions of each other, correct?
 

 

For me the high level bit is, if I've code reviewed a commit and told it to submit, I don't want to have to go back to that change and take any more action, unless a new patchset is sent in by a committer, or there is some kind of merge conflict.

As I said above, I think this is not impossible. We would need to have the merge queue code treat "submittable and rebase-if-necessary" just like it does "submitted but not merged", i.e. consider all such changes when processing a merge on the branch.

I'd be happy to help with this if you or someone could point me towards the correct code area,  I have a potential fix for http://code.google.com/p/gerrit/issues/detail?id=2167&colspec=ID%20Type%20Stars%20Milestone%20Status%20Priority%20Owner%20Summary that I have been waiting to post, but I am blocked by "fatal: remote error:  A Contributor Agreement must be completed before uploading.", despite using both the online CLA submission, and physical PDF submissions sent in on Sunday... do you know how long it takes to process those?

Thanks,
David