OpenStack Core Infrastructure

No way to tell when a change needs reapproval

Bug #1012730 reported by Thierry Carrez
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Core Infrastructure
Fix Released
High
James E. Blair
OpenStack Core Infrastructure folsom
Zuul
Fix Released
High
James E. Blair

Bug Description

We used to have the possibility to retrigger false-negative changes, but now the procedure involves asking a core reviewer to reapprove it.

However there is no way to tell that a change needs reapproval, since gerrit shows exactly the same state whether reapproval was done or not:

Status: Review in Progress
Checkmark in Approved column by <original approver>

If the reapprover is the same as the orginal approver, reapproval even does not appear in comments: for example in https://review.openstack.org/#/c/8394/ Dean reapproved the tests once and it did not appear.

This results in anger, fury, and a lot of unnecessary pings.

Would be great if you could tell that your review is actually approved/queued for testing.

Revision history for this message
Thierry Carrez (ttx) wrote :

I think the confusion mostly comes from the status of the change still showing "review in progress" while the review is no longer in progress and the change is actually waiting for tests/merge to happen.

Revision history for this message
James E. Blair (corvus) wrote :

Here's a simple potential solution:

Have Zuul clear the Jenkins verified vote when it starts running gate tests. Then you can see at a glance the status.

Verified=0, Approved=1 means jenkins is running your tests.
Verified=+/-2, Approved=1 means tests passed or failed, but are not running.

Also, having a Zuul status page might be useful in general.

Changed in openstack-ci:
status: New → Triaged
importance: Undecided → High
Changed in zuul:
status: New → Triaged
importance: Undecided → High
assignee: nobody → James E. Blair (corvus)
Changed in openstack-ci:
assignee: nobody → James E. Blair (corvus)
milestone: none → folsom
Revision history for this message
James E. Blair (corvus) wrote :

To expand on ttx's comment, we currently are using a review category to trigger merges. It really should be more of a state, along with a corresponding state transition button. However, this is what we could easily do without making source level changes to Gerrit. We could improve the situation my making a patch to gerrit to add a "Pending merge" state, use a button to transition to it, and have it exit the state back to ready for review if gate tests fail.

That would be very nice, but a slightly longer endeavor.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to zuul (master)

Fix proposed to branch: master
Review: https://review.openstack.org/8509

Changed in zuul:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to zuul (master)

Reviewed: https://review.openstack.org/8509
Committed: http://github.com/openstack-ci/zuul/commit/dc2538693e1390b2f43934f4c40d0d861fe7e308
Submitter: Jenkins
Branch: master

commit dc2538693e1390b2f43934f4c40d0d861fe7e308
Author: James E. Blair <email address hidden>
Date: Wed Jun 13 17:12:42 2012 -0700

    Add start actions.

    Add the ability to specify a report to gerrit on start. This
    can be used to clear the verified column.

    Fixes bug #1012730.

    Change-Id: I8dd2a60c3a16a8fa0046675437c750948af99577

Changed in zuul:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to openstack-ci-puppet (master)

Fix proposed to branch: master
Review: https://review.openstack.org/8554

Changed in openstack-ci:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to openstack-ci-puppet (master)

Reviewed: https://review.openstack.org/8554
Committed: http://github.com/openstack/openstack-ci-puppet/commit/ef13a895e0e23739358a456015809f667fa1891a
Submitter: Jenkins
Branch: master

commit ef13a895e0e23739358a456015809f667fa1891a
Author: James E. Blair <email address hidden>
Date: Thu Jun 14 10:20:03 2012 -0700

    Use the new Zuul start action.

    Fixes bug #1012730.

    Change-Id: Ie58882dd72a9fc5de5926c8f9889553ae82234c2

Changed in openstack-ci:
status: In Progress → Fix Released
James E. Blair (corvus)
Changed in zuul:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.