Parent and Child reviews with changes

553 views
Skip to first unread message

Michel Donais

unread,
Jul 4, 2018, 8:20:29 AM7/4/18
to Repo and Gerrit Discussion
I got into a situation where a parent Gerrit review (2.15.2) had to go through multiple revision. Same for child review. They both got approved, and when I tried pushing the Submit button on the child review, which, according to the blue button, was supposed to submit both. Instead, none got submitted and I got an error message.

The situation seems to be the child is not pointing to the proper parent revision anymore.

I understand it might be annoying queries to do, but it might be a good idea to at least mitigate the issue by inhibiting the submit button, or, at best, allow it to easily (single-button) rebase to the latest changes when the situation is discovered, instead of a cryptic error message and a gray submit button saying there was an error.

Cheers
Michel

Matthew Webber

unread,
Jul 4, 2018, 8:47:47 AM7/4/18
to Repo and Gerrit Discussion
When you say "parent" and "child", I presume you are referring to two separate changes, one of which has the other as parent.

As far as I'm aware, the behaviour you're asking for is how it already works. The submit button is only enabled when the change can actually be submitted. There is a Rebase button which can be used to rebase changes. It's only visible if you have permission to do so, of course.

It would make it easier for someone to assist if you can describe the actual sequence with more specific details - what was the error message, etc. Maybe screenshots. You post makes it sound like you might be new to Gerrit (apologies if you're not), so it may be that there is some mismatch between what you expect, and what the UI shows (and how it shows it). Were you using the PolyGerrit (new) or old-style web interface, by the way?

Michel Donais

unread,
Jul 4, 2018, 9:32:28 AM7/4/18
to Repo and Gerrit Discussion
Yes, these are two separate changes. This was with the old interface, not the new PolyGerrit UI. Sorry if I don't use the proper terminology for Gerrit, mostly created an account here to discuss this, after looking it up, it didn't seem to work through. No, I'm sorry, I should've taken screen shots, but I didn't, it's a single occurrence, and it "usually" works in simpler cases, but now it didn't. For my xp, I am definitely not new to Git and CVSes in general, but I am a Gerrit "user", not more. First months I'm using it.

Our settings require two acceptance: one by a reviewer (+2 Review) and one by the automatic verification (+1 Verified). I am the reviewer. The original submitter created two issues to be fixed, one the parent (2482) of the other (2491).

Yesterday, I did the code review for the 2482 (Parent), which required 4 different patch sets. The last one uploaded at 16:25, subsequently Verified and got accepted by me at 7:04 this AM.

Also yesterday, I did the code review for the 2491 (Child), which required 3 different patch sets. The last one was uploaded at 17:50, subsequently Verified, and got accepted by me at 7:04 this AM too.

Then, I went to the Child and I got the special button saying I can submit both the Parent and Child together, which I pressed. And I got greeted by a white page with an error message saying it could not submit the parent. I went back through my browser's back button, and the button was grayed out with a mention the current patch set failed to submit.

To fix the issue, I submitted the 2482 Parent, then, I Rebased the second one, and Submitted it.

From what I see by selecting the different patch sets of the Child, the parent is always selected as the 2nd patch set of the Parent, which makes sense since it got sent in review after the 2nd patch set of the parent. But the patch sets 3 and 4 of the Parent never got reparented into the Child.

Björn Pedersen

unread,
Jul 5, 2018, 4:18:19 AM7/5/18
to Repo and Gerrit Discussion
Hi,

this is probably mostly dependent on some of the settings for submit in this case (from what you mention I assume you are using the default "merge if necessay" strategy):
   In the project settings, check if 'content merge' is enabled, otherwise even minor conflicts may casue such troubles.


The other thing is client-side workflow:

To avoid such problems, you should work like:
  create local branch
  create commit-1
  create commit-2
  push...
  rework (via rebase -i) commit 1 or commit 2
  push...

This way, the parent-child relationship stays intact.
 
Björn

Michel Donais

unread,
Jul 5, 2018, 10:59:24 AM7/5/18
to Repo and Gerrit Discussion
Thank you for the answer,

from the settings perspective, we are pretty much always using the inherited settings. The submit type is "Rebase if necessary" and the Allow content merges is "true".

For the client-side workflow, I will ask the submitter if it fits the mould of what he did.

Thanks
Michel

Samuel Longchamps

unread,
Jul 6, 2018, 2:23:23 AM7/6/18
to Repo and Gerrit Discussion
I'm the submitter for this particular case. My use case was basically to be able to work on a dependent task in a second commit while doing aesthetic changes on the first commit, allowing me to build both concurrently.

What I did is:
  create local branch-task1
  create commit-1
  create local branch-task2 (from branch-task1)
  create commit-2
  publish for review both commits

Then, I did the changes in each branch on their respective HEAD. Since our strategy is "Rebase if Necessary", I assumed that on a situation where no merge conflicts would occur, submitting the second review would automatically rebase on the latest patchset of the dependent review: otherwise, if that were not possible, indicate "Cannot Merge".

Also, I understand the workflow Björn suggested, but it was more cumbersome to maintain a branch with the latest patchsets when I expect Gerrit to be the one doing the rebase work when it's trivial (no conflicts). Perhaps our expectations of trivial is too broad (of course, one could expect this rebase to yield a patchset that would need to be tested and reviewed again).

It'd be nice to have a view of the dependency tree of change-ids in the review interface. Something like the related changes, but more as an ordered list from old to new of open reviews.

On Thursday, July 5, 2018 at 4:18:19 AM UTC-4, Björn Pedersen wrote:
Reply all
Reply to author
Forward
0 new messages