New issue
Advanced search Search tips

Issue 734260 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----


Previous locations:
gerrit:6497


Sign in to add a comment

Non-committer merge workflow (drover) broken with Gerrit

Project Member Reported by amp@chromium.org, Jun 13 2017

Issue description

With Reitvald, a non-committer can run drover and, while it won't be able to land the change, create a review to merge the change to a specific branch.  Once a committer lgtm's that change it can be landed and shows up on the branch as if the non-committer was the author.

(Note that having a commiter run drover directly for a specific change works as well, but the author of the change on the branch will show up as the committer, not the original non-committer author).

With Gerrit, a change is created, but there is no commit queue access allowed to land the change, even after a committer lgtm's the change (and apparently even the committer doesn't have access to +2 commit-queue the change).

This is low priority, but it would be good to clarify what the expected behavior is in these scenario's.

An example to illustrate:
Original change - https://codereview.chromium.org/2930113002
Gerrit attempt at merging - https://chromium-review.googlesource.com/c/534013/
Final merge - https://codereview.chromium.org/2933923003/


 

Comment 1 by logan@google.com, Jun 16 2017

Components: -PolyGerrit
Labels: Proj-Gerrit-Migration
Sending over the chrome-infra to triage.

Comment 2 by logan@google.com, Jun 16 2017

Project: chromium
Moved issue gerrit:6497 to now be  issue chromium:734260 .

Comment 3 by kezcy...@gmail.com, Jun 16 2017

Labels: -Proj=Gerrit-Migration Proj-Gerrit-Migration

Comment 4 by aga...@chromium.org, Jun 20 2017

Components:
Labels: Milestone-Afterglow
Owner: aga...@chromium.org
Status: Assigned (was: New)
Thanks for the heads up. This seems to be a simple misconfiguration. Because the release branches don't have a "real" CQ (no tryjobs get run), we didn't enable the Commit-Queue label on the release branches. I'll look into enabling the CQ label there.

Comment 5 by aga...@chromium.org, Jun 26 2017

Status: Fixed (was: Assigned)
So it turns out the resolution here is not to enable the CQ label on release branches, but to disable Chumpdetector on release branches. The Rietveld CQ had some terrible hacks built in that allowed it to land changes on release branches under certain very specific circumstances. But it didn't do any presubmit checks, didn't run any tests, and didn't really do anything that would make it "the CQ".

So, with Gerrit, we've removed that. The CQ label can't be set on changes to release branches, and the Submit button (or git-cl-land) should be used to submit changes to them.

The one problem with the above was that the Chumpdetector would still yell at you if you clicked the Submit button. I've disabled the chumpdetector on release branches now, so that mixed-messaging is now gone.

Sign in to add a comment