Open Stack

On Mon, Jul 20, 2015 at 12:24:13PM -0600, Matt Fischer wrote:
>Dmitry and I chatted some this morning and I'd at least like to address
>those issues so that we can resolve them and move on to the other
>discussions. I am not speaking for Emilien here, just jumping in as a core
>and trying to help resolve some of this.
>
>As Emilien notes, many of us do not work full-time on puppet code, so
>sometimes reviews can slip. IRC messages can also be missed especially
>during summer months with many of us on vacations. For that reason, Dmitry
>and the Fuel team will be bringing reviews to the weekly meeting to call
>out ones that need or are overdue for attention. I believe that missed,
>overlooked, ignored, or forgotten reviews are not a new issue for an
>OpenStack project, but hopefully this solution will work to improve
>throughput. This solution applies to anyone who is possibly not getting
>enough attention on the review.
>
>Our review dashboard is here: https://goo.gl/bSYJt8

After looking carefully at this dashboard's definition, I think I've 
figured out what happened to https://review.openstack.org/190548 -- the 
dashboard's foreach statement excludes all reviews with a -2 code review 
vote, so once that review received a -2 from Emilien, it would no longer 
show up in anyone's inbox, including Emilien's.

To mitigate that, I've proposed a change for the dashboard that adds a 
"Disagreement" section including reviews with both positive and negative 
votes:

https://review.openstack.org/203903

The rationale is that if, despite someone voting a patch set (or 
the whole review) down, other reviewers did not revoke (or kept adding) 
positive votes, further discussion of that commit is needed. It would 
still be possible to exclude a commit from all sections of the 
dashboard, if necessary (e.g. author is MIA) by abandoning it.

Reviewing that list once a week might be a good way to put stuck reviews 
on the meeting's agenda, and would benefit all contributors, not only 
Fuel developers.

Thoughts?

-- 
Dmitry Borodaenko