Last modified: 2013-03-20 13:36:19 UTC
See the comment history for https://gerrit.wikimedia.org/r/#/c/49187/ The last action I took was to comment and CR+2, then stepped away to actually verify. When I returned to enter my verification and merge, the jenkins-bot had already Verified+2 and merged without my interaction.
This is by design? Moving to testing infrastructure, as it's not Git/Gerrit, but rather Jenkins/Zuul.
Voting CR+2 is similar to approving the change to land in master. That triggers a job that run the tests and merge the change whenever they have been successful. We can disable the automatic merge if you prefer. That would just run the unit tests and report the result.
Is there a config setting per project? I wouldn't want to change the default for other repositories without a community discussion. FWIW, I found Chad's original announcement of this feature here, http://www.gossamer-threads.com/lists/wiki/wikitech/321317 -- I guess the confusion is caused by not having a precise definition of "Verified+2"? Anyway, IMO a bot should never merge, there should be explicit user action.
(In reply to comment #3) > Is there a config setting per project? I wouldn't want to change the default > for other repositories without a community discussion. FWIW, I found Chad's > original announcement of this feature here, > http://www.gossamer-threads.com/lists/wiki/wikitech/321317 -- I guess the > confusion is caused by not having a precise definition of "Verified+2"? > > Anyway, IMO a bot should never merge, there should be explicit user action. The explicit user action is "CR+2". Don't be fooled by terminology ("review" and "merge"). The design is that things should be tested before merging. And since we can't hook (yet?) hook into the "Merge" button in Gerrit, we instead hook into the "CR+2" submission button, and let Jenkins merge it. So basically it is this: - User reviews - User approves merge - Bot tests code - Bot lets merge happen or aborts it Similar to how an extension can hook into an action in MediaWiki, and if the hook returns false it action is aborted, and if it returns true it is allowed. However, due to the way Gerrit and Zuul work, you need to be aware of how these buttons work. So while our intention is this: * Users does CR -2, -1, 0, +1, +2 * User does Merge. Only merge if you intend to merge, otherwise leave comment with CR+2. * If no CR+2, abort * On Merge: Run tests, if fail, disallow merge, otherwise continue The actual buttons to be used are like this: * Users does CR -2, -1, 0, +1, +2 * User does CR+2. Only CR+2 if you intend to merge, otherwise leave comment with CR+1. * On CR+2: Run tests, if fail, score V-2, otherwise merge This is naturally confusing when not familiar with this fact. Which is +2 is only available to people who are expected to know this, and even of those people most people do not have access to the form for "Verified" score (just Code-Review score), and only have a "Publish score/comments" button, not a "Submit change" (merge) button.
(In reply to comment #3) > Is there a config setting per project? I wouldn't want to change the default > for other repositories without a community discussion. FWIW, I found Chad's > original announcement of this feature here, > http://www.gossamer-threads.com/lists/wiki/wikitech/321317 -- I guess the > confusion is caused by not having a precise definition of "Verified+2"? > > Anyway, IMO a bot should never merge, there should be explicit user action. The explicit user action is "CR+2". Don't be fooled by terminology ("review" and "merge"). The design is that things should be tested before merging. And since we can't (yet?) hook into the "Merge" button in Gerrit, we instead hook into the "CR+2" submission button, and let Jenkins merge it. So basically it is this: - User reviews - User approves merge - Bot tests code - Bot lets merge happen or aborts it Similar to how an extension can hook into an action in MediaWiki, and if the hook returns false the action is aborted, and if it returns true it is allowed. However, due to the way Gerrit and Zuul work, you need to be aware of how these buttons work. So while our intention is this: * User does CR -2, -1, 0, +1, +2 * User does Merge. Only merge if you intend to merge, otherwise leave comment with CR+2. * If no CR+2, abort * On Merge: Run tests, if fail, disallow merge, otherwise continue The actual buttons to be used are like this: * User does CR -2, -1, 0, +1, +2 * User does CR+2. Only CR+2 if you intend to merge, otherwise leave comment with CR+1. * On CR+2: Run tests, if fail, score V-2, otherwise merge This is naturally confusing when not familiar with this fact. Which is +2 is only available to people who are expected to know this, and even of those people most people do not have access to the form for "Verified" score (just Code-Review score), and only have a "Publish score/comments" button, not a "Submit change" (merge) button.
> Anyway, IMO a bot should never merge, there should be explicit user action. Which is to vote CR+2. The bot is merely an helper that run tests for you and submit the change. Just list the repositories for which you would prefer Jenkins to not merge and I will be more than happy to change the behavior.
Closing bug as working for me, that is the expecting behavior. It can be disabled on certain repositories by editing Zuul configuration, simply need to remove the gate-and-submit: pipeline from the project.
