[openstack-dev] [Glance] Process to clean up the review queue from non-active patches

Bunting, Niall niall.bunting at hpe.com
Wed Oct 7 12:47:08 UTC 2015


> From: Julien Danjou [julien at danjou.info]
> Sent: 07 October 2015 10:12
> 
> On Wed, Oct 07 2015, Flavio Percoco wrote:
> 
> > I'm not trying to solve the lack of reviews in Liberty by removing
> > patches. What I'd like to do, though, is help to keep around patches
> > that really matter.
> 
> I think that's where you are making a mistake. They are contributors,
> like me or Victor, are knocking on the Glance doors for months now,
> sending patches that resolve technical debt rather than adding new
> debt^Wfeatures. Currently, these patches are not seen as important and
> are often dismissed. So I'm pretty sure they are going to expire with
> this new system.
> 
> Imagine that if you were merging patches from me, Victor, and people
> like us, we would continue to send many of them, and mid-term, you'd get
> some new blood on your core team.
> 
> What is proposed here is really focusing on making life easier for the
> current core team which is in large majority inactive.
> 
> Don't read me wrong. I know you and Nikhil are both well-intentioned by
> proposing that. I just think it's going to be worse, because it won't
> improve much and you're going to push new contributors away.

If your patches are sitting there waiting for review once they get off
the top couple of pages they are likely to become buried, as they are
waiting for review. This is unlikely to benefit anyone.

How would an active user keep their patches it the active/up for review
state? By just posting bump comments?

Any type of change Juilen is talking about could keep on being dismissed,
and this could end up in some sort of game to keep your patch above
the non-review line for it just to be ignored. This would definitely be
a bad thing, therefore I think any patches that are picked up as old,
should be reviewed before being marked as WIP. As this would mean if
the patch is moved out of WIP it would be less likely to get stuck in
some sort of loop as it has a review.

We also have to be careful about alienating contributors, we should make
sure they know why there work got marked WIP with a link to a wiki page
explaining the process. However if this forces a review, they may also
be happy that they eventually got a review on their patch.

My thoughts,
Niall

Edit: As this did not send to the list originally.
Flavio points out that they aim to review patches that are unmarked wip,
I think that system could work as long as it avoids the problem of patches
potentially becoming stuck in a circle.

And It should be kept in mind that even old reviews could still be relevant,
and the best course of action may not to just mark them wip without thought.


More information about the OpenStack-dev mailing list