Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 07/10/15 10:17 -0400, Doug Hellmann wrote: Excerpts from Flavio Percoco's message of 2015-10-07 16:50:16 +0900: On 06/10/15 23:36 +0900, Flavio Percoco wrote: >Greetings, > >Not so long ago, Erno started a thread[0] in this list to discuss the >abandon policies for patches that haven't been updated in Glance. > >I'd like to go forward and start following that policy with some >changes that you can find below: > >1) Lets do this on patches that haven't had any activity in the last 2 >months. This adds one more month to Erno's proposal. The reason being >that during the lat cycle, there were some ups and downs in the review >flow that caused some patches to get stuck. > >2) Do this just on master, for all patches regardless they fix a >bug or implement a spec and for all patches regardless their review >status. > >3) The patch will be first marked as a WIP and then abandoned if the >patch is not updated in 1 week. This will put this patches at the >begining of the queue but using the Glance review dashboard should >help keeing focus. > >Unless there are some critical things missing in the above or strong >opiniones against this, I'll make this effective starting next Monday >October 12th. I'd like to provide some extra data here. This is our current status: == Total patches without activity in the last 2 months: 73 Total patches closing a bug: 30 Total patches with negative review by core reviewers: 62 Total patches with negative review by non-core reviewers: 75 Total patches without a core review in the last patchset: 13 Total patches with negative review from Jenkins: 50 == It's not ideal but it's also not a lot. I'd like to recover as many patches as possible from the above and I'm happy to do that manually if necessary. Cheers, Flavio It might be useful to schedule a review sprint to take a couple of days to focus on reviews. Maybe the team can pick dates a couple of weeks in advance so everyone can get permission to spend the full time on reviews. +1 This is something I've planned. 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
Excerpts from Flavio Percoco's message of 2015-10-07 16:50:16 +0900: > On 06/10/15 23:36 +0900, Flavio Percoco wrote: > >Greetings, > > > >Not so long ago, Erno started a thread[0] in this list to discuss the > >abandon policies for patches that haven't been updated in Glance. > > > >I'd like to go forward and start following that policy with some > >changes that you can find below: > > > >1) Lets do this on patches that haven't had any activity in the last 2 > >months. This adds one more month to Erno's proposal. The reason being > >that during the lat cycle, there were some ups and downs in the review > >flow that caused some patches to get stuck. > > > >2) Do this just on master, for all patches regardless they fix a > >bug or implement a spec and for all patches regardless their review > >status. > > > >3) The patch will be first marked as a WIP and then abandoned if the > >patch is not updated in 1 week. This will put this patches at the > >begining of the queue but using the Glance review dashboard should > >help keeing focus. > > > >Unless there are some critical things missing in the above or strong > >opiniones against this, I'll make this effective starting next Monday > >October 12th. > > I'd like to provide some extra data here. This is our current status: > > == > Total patches without activity in the last 2 months: 73 > Total patches closing a bug: 30 > Total patches with negative review by core reviewers: 62 > Total patches with negative review by non-core reviewers: 75 > Total patches without a core review in the last patchset: 13 > Total patches with negative review from Jenkins: 50 > == > > It's not ideal but it's also not a lot. I'd like to recover as many > patches as possible from the above and I'm happy to do that manually > if necessary. > > Cheers, > Flavio > It might be useful to schedule a review sprint to take a couple of days to focus on reviews. Maybe the team can pick dates a couple of weeks in advance so everyone can get permission to spend the full time on reviews. Doug __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 07/10/15 12:47 +, Bunting, Niall wrote: From: Julien Danjou [jul...@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? 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
> From: Julien Danjou [jul...@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. __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 07/10/15 11:12 +0200, Julien Danjou wrote: 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. I can't do anything for the past failures other than saying I'm sorry. As I mentioned in previous emails in this thread, the work to make the review process better is unrelated to the topic on this email, really. I wouldn't say that your patches (or Victor's) weren't important for the team but I would like to avoid getting into the details of the past, tbh. 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. I don't think this needs to be explained and I trust the whole Glance core team to know this. Although, it's better to be explicit than implicit so, thanks. What is proposed here is really focusing on making life easier for the current core team which is in large majority inactive. This is were I think I'm failing to communicate the intention here. The dashboard[0] I've put up is the one that intends to make the core team's life easier. [0] http://bit.ly/glance-review-dashboard 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. Absolutely, I value everyone's feedback on this thread a lot. I hope I'm explaining the goal correctly. If I'm not, I'm more than happy to talk more about this (See my email with some stats for example). Cheers, 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
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. -- Julien Danjou # Free Software hacker # https://julien.danjou.info 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 06/10/15 23:36 +0900, Flavio Percoco wrote: Greetings, Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. I'd like to go forward and start following that policy with some changes that you can find below: 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. 2) Do this just on master, for all patches regardless they fix a bug or implement a spec and for all patches regardless their review status. 3) The patch will be first marked as a WIP and then abandoned if the patch is not updated in 1 week. This will put this patches at the begining of the queue but using the Glance review dashboard should help keeing focus. Unless there are some critical things missing in the above or strong opiniones against this, I'll make this effective starting next Monday October 12th. I'd like to provide some extra data here. This is our current status: == Total patches without activity in the last 2 months: 73 Total patches closing a bug: 30 Total patches with negative review by core reviewers: 62 Total patches with negative review by non-core reviewers: 75 Total patches without a core review in the last patchset: 13 Total patches with negative review from Jenkins: 50 == It's not ideal but it's also not a lot. I'd like to recover as many patches as possible from the above and I'm happy to do that manually if necessary. Cheers, Flavio -- @flaper87 Flavio Percoco pgpEt3dCWEcon.pgp 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 06/10/15 13:53 -0400, Doug Hellmann wrote: Excerpts from Flavio Percoco's message of 2015-10-06 23:36:53 +0900: Greetings, Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. I'd like to go forward and start following that policy with some changes that you can find below: 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. 2) Do this just on master, for all patches regardless they fix a bug or implement a spec and for all patches regardless their review status. 3) The patch will be first marked as a WIP and then abandoned if the patch is not updated in 1 week. This will put this patches at the begining of the queue but using the Glance review dashboard should help keeing focus. Unless there are some critical things missing in the above or strong opiniones against this, I'll make this effective starting next Monday October 12th. Best regards, Flavio [0] http://lists.openstack.org/pipermail/openstack-dev/2015-February/056829.html In the past we've had discussions on the list about how abandoning patches can be perceived as hostile to contributors, and that using a review dashboard with good filters is a better solution. Since you already have a dashboard, I suggest adding a section for patches that are old but have no review comments (maybe you already have that) and another for patches where the current viewer has voted -1. The first highlights the patches for reviewers, and ignores them when they are in a state where we're waiting for feedback or an update, and the latter provides a list of patches the current reviewer is involved in and may need to recheck for new comments. We definitely don't want anyone to feel bad about this, especially when it's our fault they patches are still hanging around. This is the reason why the WIP phase is being proposed as away to ask the user to come to use and remind us to do our job. Not ideal, sure, but it happens to everyone. The current dashboard has that section already and I really hope we don't get to the point where abandoning patches ourselves is necessary. The section is called "5 Days Without Feedback" and I gotta be honest, it's worked (the dashboard in general) very well for me and I hope for others as well. There are patches from old contributors that we just know are never going to be worked out. These patches I'd like to take a look closely before abandoning them so that we can rebase them ourselves if they are still relevant. Making this list shorter is what I'd like to achieve with the proposed plan. Hope the above makes sense, Flavio -- @flaper87 Flavio Percoco pgpSJOU3hsy5w.pgp 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 06/10/15 17:54 +0200, Victor Stinner wrote: Hi, Le 06/10/2015 16:36, Flavio Percoco a écrit : Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. (...) 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. Please don't do that. I sent a patch in June (20) and it was only reviewed in October (4)... There was no activity simply because I had nothing to add, everything was explained in the commit message, I was only waiting for a review... I came on #openstack-glance to ask for review several time between August and September but nobody reviewed by patches (there was al. Example of patch: https://review.openstack.org/#/c/193786/ (now merged) Yes, I'm very aware of this case and it's great feedback. What happened with these patches could have (or did happen) with other patches. The review response is something that I'd definitely like us to improve and this is not the solution. It would be very frustrating to have to resend the same patch over and over. There's no need to resend the patch. Just commenting saying that it's still a valid update and it'll keep the patch from being abandoned. Cheers, Flavio -- @flaper87 Flavio Percoco pgpmF99M4Fd9v.pgp 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 06/10/15 12:11 -0400, Nikhil Komawar wrote: Overall I think this is a good idea and the time frame proposal also looks good. Few suggestions in-line. On 10/6/15 10:36 AM, Flavio Percoco wrote: Greetings, Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. I'd like to go forward and start following that policy with some changes that you can find below: 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. +2 . I think 2 months is a reasonable time frame. Though, I think this should be done on glance , python-glanceclient and glance-store repos and not glance-specs. Specs can sometimes need to sit and wait while discussion may happen at other places and then a gist is added back the spec. Yup, no plans to apply this to glance-specs, just code. Thanks for the feedback, Flavio 2) Do this just on master, for all patches regardless they fix a bug or implement a spec and for all patches regardless their review status. +2 . No comments, looks clean. 3) The patch will be first marked as a WIP and then abandoned if the patch is not updated in 1 week. This will put this patches at the begining of the queue but using the Glance review dashboard should help keeing focus. While I think that one may give someone a email/irc heads up if the proposer doesn't show up and we will use the context and wisdom of feedback this sorta seems to imply for a general case when a developer is new and their intent to get a patch in one cycle isn't clear. Unless there are some critical things missing in the above or strong opiniones against this, I'll make this effective starting next Monday October 12th. I added some comments above for possible brainstorming. No serious objections, looking forward to this cleanup process. Best regards, Flavio [0] http://lists.openstack.org/pipermail/openstack-dev/2015-February/ 056829.html __ 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 -- Thanks, Nikhil -- @flaper87 Flavio Percoco pgpK_pi2Tj7Yr.pgp 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 06/10/15 17:52 +0200, Julien Danjou wrote: On Tue, Oct 06 2015, Flavio Percoco wrote: I send patches to Glance from time to time, and they usually got 0 review for *weeks* (sometimes months, because, well there are no reviewers active in Glance, so: 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. This is going to expire my patches that nobody cares about and that are improving the code or fixing stuff people didn't encounter (yet). 3) The patch will be first marked as a WIP and then abandoned if the patch is not updated in 1 week. This will put this patches at the begining of the queue but using the Glance review dashboard should help keeing focus. Why WIP? If a patch is complete and waiting for reviewers I'm not sure it helps. WIP because I don't think we should abandon them right away - since there are patches like yours and Victor's that matter - and there's no status to say: "I'm sorry we screwed up and we didn't review your patch. Please come to us and throw all your amazing patches in our faces so that we'll review them... for realz" The problem is that nobody is reviewing Glance patches (except you recently it seems). That's not going to solve that. That's just going to hide the issues under the carpet by lowering the total of patches that needs review… 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 know there have been a huge lag on reviews which is something that we'll be working on with a different workflow. The dashboard mentioned is one of them. We could certainly increase the number of months. Thanks a lot for the feedback, Flavio My 2c, -- Julien Danjou ;; Free Software hacker ;; https://julien.danjou.info -- @flaper87 Flavio Percoco pgpDx8liLHnv6.pgp 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On 10/6/15 1:53 PM, Doug Hellmann wrote: > Excerpts from Flavio Percoco's message of 2015-10-06 23:36:53 +0900: >> Greetings, >> >> Not so long ago, Erno started a thread[0] in this list to discuss the >> abandon policies for patches that haven't been updated in Glance. >> >> I'd like to go forward and start following that policy with some >> changes that you can find below: >> >> 1) Lets do this on patches that haven't had any activity in the last 2 >> months. This adds one more month to Erno's proposal. The reason being >> that during the lat cycle, there were some ups and downs in the review >> flow that caused some patches to get stuck. >> >> 2) Do this just on master, for all patches regardless they fix a >> bug or implement a spec and for all patches regardless their review >> status. >> >> 3) The patch will be first marked as a WIP and then abandoned if the >> patch is not updated in 1 week. This will put this patches at the >> begining of the queue but using the Glance review dashboard should >> help keeing focus. >> >> Unless there are some critical things missing in the above or strong >> opiniones against this, I'll make this effective starting next Monday >> October 12th. >> >> Best regards, >> Flavio >> >> [0] >> http://lists.openstack.org/pipermail/openstack-dev/2015-February/056829.html >> > In the past we've had discussions on the list about how abandoning > patches can be perceived as hostile to contributors, and that using > a review dashboard with good filters is a better solution. Since > you already have a dashboard, I suggest adding a section for patches > that are old but have no review comments (maybe you already have > that) and another for patches where the current viewer has voted > -1. The first highlights the patches for reviewers, and ignores > them when they are in a state where we're waiting for feedback or > an update, and the latter provides a list of patches the current > reviewer is involved in and may need to recheck for new comments. That hostility aspect is the main reason why abandoning has been avoided until now. > Doug > > __ > 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 -- Thanks, Nikhil __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
Excerpts from Flavio Percoco's message of 2015-10-06 23:36:53 +0900: > Greetings, > > Not so long ago, Erno started a thread[0] in this list to discuss the > abandon policies for patches that haven't been updated in Glance. > > I'd like to go forward and start following that policy with some > changes that you can find below: > > 1) Lets do this on patches that haven't had any activity in the last 2 > months. This adds one more month to Erno's proposal. The reason being > that during the lat cycle, there were some ups and downs in the review > flow that caused some patches to get stuck. > > 2) Do this just on master, for all patches regardless they fix a > bug or implement a spec and for all patches regardless their review > status. > > 3) The patch will be first marked as a WIP and then abandoned if the > patch is not updated in 1 week. This will put this patches at the > begining of the queue but using the Glance review dashboard should > help keeing focus. > > Unless there are some critical things missing in the above or strong > opiniones against this, I'll make this effective starting next Monday > October 12th. > > Best regards, > Flavio > > [0] > http://lists.openstack.org/pipermail/openstack-dev/2015-February/056829.html > In the past we've had discussions on the list about how abandoning patches can be perceived as hostile to contributors, and that using a review dashboard with good filters is a better solution. Since you already have a dashboard, I suggest adding a section for patches that are old but have no review comments (maybe you already have that) and another for patches where the current viewer has voted -1. The first highlights the patches for reviewers, and ignores them when they are in a state where we're waiting for feedback or an update, and the latter provides a list of patches the current reviewer is involved in and may need to recheck for new comments. Doug __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
This again becomes a part of the undefinied priority concept. It's hard to come up with a list of priorities all at the beginning of the cycle so, conflicting priority items may get missed every now and then. I hope the dashboard will help however, this is more of a people problem than a process problem what Victor is describing. To solve that, here's the context: If a change is either big, has a spec associated with it or doesn't have a bug associated it is unlikely to catch the notice of the cores easily let alone other reviewers. Something that would help is to propose a topic to the weekly meeting and if you can't attend delegate it to someone who will -- I have seen it help catch attention of the cores. Specs and other big changes are often discussed at the drivers meeting so that's a good place to bring this up too. It doesn't have to be a long one, just a heads up suffices many a times. On 10/6/15 11:54 AM, Victor Stinner wrote: > Hi, > > Le 06/10/2015 16:36, Flavio Percoco a écrit : >> Not so long ago, Erno started a thread[0] in this list to discuss the >> abandon policies for patches that haven't been updated in Glance. >> (...) >> 1) Lets do this on patches that haven't had any activity in the last 2 >> months. This adds one more month to Erno's proposal. The reason being >> that during the lat cycle, there were some ups and downs in the review >> flow that caused some patches to get stuck. > > Please don't do that. I sent a patch in June (20) and it was only > reviewed in October (4)... There was no activity simply because I had > nothing to add, everything was explained in the commit message, I was > only waiting for a review... > > I came on #openstack-glance to ask for review several time between > August and September but nobody reviewed by patches (there was al. > > Example of patch: https://review.openstack.org/#/c/193786/ (now merged) > > It would be very frustrating to have to resend the same patch over and > over. > > Victor > > __ > > 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 -- Thanks, Nikhil __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
I think Glance reviewer brandwidth is pretty low and the feedback time can be quite high. For specs, I had requested a section describing the core reviewer who will be vouching for your spec to be added to the spec itself. I think in general we have not seen anyone doing that strongly. If a spec isn't proposed as a priority at the beginning of a summit and if it's a big change and is not decided to an extent, it becomes quite tricky to get in the project in that cycle. As a general feedback I have always recommend bringing feature proposals at summit sessions, mid-cycle, meetings etc etc. If a feature has missed it other proposal have gotten priority. Also, there were priorities that were set the beginning of the cycle and if a proposal isn't on the list there, it becomes tricky to make a call for a new spec to get merged in one cycle. Happy to help on the process, review speed, activities 'context' more if you want input. On 10/6/15 11:52 AM, Julien Danjou wrote: > On Tue, Oct 06 2015, Flavio Percoco wrote: > > I send patches to Glance from time to time, and they usually got 0 > review for *weeks* (sometimes months, because, well there are no > reviewers active in Glance, so: > >> 1) Lets do this on patches that haven't had any activity in the last 2 >> months. This adds one more month to Erno's proposal. The reason being >> that during the lat cycle, there were some ups and downs in the review >> flow that caused some patches to get stuck. > This is going to expire my patches that nobody cares about and that are > improving the code or fixing stuff people didn't encounter (yet). > >> 3) The patch will be first marked as a WIP and then abandoned if the >> patch is not updated in 1 week. This will put this patches at the >> begining of the queue but using the Glance review dashboard should >> help keeing focus. > Why WIP? If a patch is complete and waiting for reviewers I'm not sure > it helps. > > The problem is that nobody is reviewing Glance patches (except you > recently it seems). That's not going to solve that. That's just going to > hide the issues under the carpet by lowering the total of patches that > needs review… > > My 2c, > > > > __ > 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 -- Thanks, Nikhil __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
Overall I think this is a good idea and the time frame proposal also looks good. Few suggestions in-line. On 10/6/15 10:36 AM, Flavio Percoco wrote: > Greetings, > > Not so long ago, Erno started a thread[0] in this list to discuss the > abandon policies for patches that haven't been updated in Glance. > > I'd like to go forward and start following that policy with some > changes that you can find below: > > 1) Lets do this on patches that haven't had any activity in the last 2 > months. This adds one more month to Erno's proposal. The reason being > that during the lat cycle, there were some ups and downs in the review > flow that caused some patches to get stuck. > +2 . I think 2 months is a reasonable time frame. Though, I think this should be done on glance , python-glanceclient and glance-store repos and not glance-specs. Specs can sometimes need to sit and wait while discussion may happen at other places and then a gist is added back the spec. > 2) Do this just on master, for all patches regardless they fix a > bug or implement a spec and for all patches regardless their review > status. > +2 . No comments, looks clean. > 3) The patch will be first marked as a WIP and then abandoned if the > patch is not updated in 1 week. This will put this patches at the > begining of the queue but using the Glance review dashboard should > help keeing focus. > While I think that one may give someone a email/irc heads up if the proposer doesn't show up and we will use the context and wisdom of feedback this sorta seems to imply for a general case when a developer is new and their intent to get a patch in one cycle isn't clear. > Unless there are some critical things missing in the above or strong > opiniones against this, I'll make this effective starting next Monday > October 12th. > I added some comments above for possible brainstorming. No serious objections, looking forward to this cleanup process. > Best regards, > Flavio > > [0] > http://lists.openstack.org/pipermail/openstack-dev/2015-February/056829.html > > > > > __ > 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 -- Thanks, Nikhil __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
Hi, Le 06/10/2015 16:36, Flavio Percoco a écrit : Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. (...) 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. Please don't do that. I sent a patch in June (20) and it was only reviewed in October (4)... There was no activity simply because I had nothing to add, everything was explained in the commit message, I was only waiting for a review... I came on #openstack-glance to ask for review several time between August and September but nobody reviewed by patches (there was al. Example of patch: https://review.openstack.org/#/c/193786/ (now merged) It would be very frustrating to have to resend the same patch over and over. Victor __ 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
Re: [openstack-dev] [Glance] Process to clean up the review queue from non-active patches
On Tue, Oct 06 2015, Flavio Percoco wrote: I send patches to Glance from time to time, and they usually got 0 review for *weeks* (sometimes months, because, well there are no reviewers active in Glance, so: > 1) Lets do this on patches that haven't had any activity in the last 2 > months. This adds one more month to Erno's proposal. The reason being > that during the lat cycle, there were some ups and downs in the review > flow that caused some patches to get stuck. This is going to expire my patches that nobody cares about and that are improving the code or fixing stuff people didn't encounter (yet). > 3) The patch will be first marked as a WIP and then abandoned if the > patch is not updated in 1 week. This will put this patches at the > begining of the queue but using the Glance review dashboard should > help keeing focus. Why WIP? If a patch is complete and waiting for reviewers I'm not sure it helps. The problem is that nobody is reviewing Glance patches (except you recently it seems). That's not going to solve that. That's just going to hide the issues under the carpet by lowering the total of patches that needs review… My 2c, -- Julien Danjou ;; Free Software hacker ;; https://julien.danjou.info 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
[openstack-dev] [Glance] Process to clean up the review queue from non-active patches
Greetings, Not so long ago, Erno started a thread[0] in this list to discuss the abandon policies for patches that haven't been updated in Glance. I'd like to go forward and start following that policy with some changes that you can find below: 1) Lets do this on patches that haven't had any activity in the last 2 months. This adds one more month to Erno's proposal. The reason being that during the lat cycle, there were some ups and downs in the review flow that caused some patches to get stuck. 2) Do this just on master, for all patches regardless they fix a bug or implement a spec and for all patches regardless their review status. 3) The patch will be first marked as a WIP and then abandoned if the patch is not updated in 1 week. This will put this patches at the begining of the queue but using the Glance review dashboard should help keeing focus. Unless there are some critical things missing in the above or strong opiniones against this, I'll make this effective starting next Monday October 12th. Best regards, Flavio [0] http://lists.openstack.org/pipermail/openstack-dev/2015-February/056829.html -- @flaper87 Flavio Percoco pgpHAjhpRRm1H.pgp 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