On 07/10/15 12:47 +0000, Bunting, Niall wrote:
From: Julien Danjou [jul...@danjou.info] Sent: 07 October 2015 10:12On 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?
No one should need to keep their reviews in the "active" queue as there's not really an active queue. There are just tons of pages. When a patch is marked as WIP because it hasn't been updated, the user just needs to comment on it to keep it around. The WIP flag will be removed then. The comment is needed because one cannot remove the WIP flag of other users in gerrit. If there are un-addressed comments, proposing a patch-set addressing these comments would be the best/right way to remove the WIP flag. Also, the WIP is not really mandatory. We could do away with it. I think it'd be useful for users as it'd be clear that something is happening in their patches when they load their personal dashboard. Many folks ignore gerrit emails and they may miss the comment update, which is what really worries me.
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.
The plan is not to just mark as WIP and be done with it. The plan is to obviously provide enough information for users to know what the next step is. I should've probably mentioned this in my original email.
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.
Again, the plan is not to just go and blindly abandon. The above is important and certainly being kept in mind. Flavio -- @flaper87 Flavio Percoco
signature.asc
Description: PGP signature
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
