Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Thu, Mar 5, 2015, at 06:59 AM, Alexis Lee wrote: Doug Hellmann said on Wed, Mar 04, 2015 at 11:10:31AM -0500: I used to use email to track such things, but I have reached the point where keeping up with the push notifications from gerrit would consume all of my waking time. Jim said if his patch was auto-abandoned, he would not find out. There are mail filtering tools, custom dashboards, the REST API. There are a myriad of ways to find out, it seems false to complain about a lack of notification if you turned them off yourself and choose not to use alternative tooling. I can certainly do all of those things, and do, but I'm becoming a more skilled gerrit user. My concern is about new contributors, who have not learned gerrit or who have not learned the way we have customized our use of gerrit. Throwing away their work because they've been away for some period of time (think extended vacation, illness, family emergency) is hostile. The patch is already in a state that would prevent it from merging, no? So if it we can hide it from reviewers that don't want to see those sorts of patches, without being hostile and without hiding it from *all* reviewers, that seems better than automatically discarding it. I'm not saying it's perfect. Gerrit could offer granular control of push notifications. It's also awkward to filter auto-abandoned patches from manually abandoned, which is why I think a new flag or two with bespoke semantics is the best solution. As Jim and others have pointed out, we can identify those changesets using existing attributes rather than having to add a new flag. This doesn't help new contributors who aren't using your custom dashboard. The defaults have to be sensible. The default dashboard must identify patches which will never be reviewed and a push notification should be available for when a patch enters that state. Does the default dashboard we have not seem sensible for a new contributor? When I'm logged in it shows me all of my open changes (this is where abandoning a change makes it disappear) and reviews where I've commented. A slightly more advanced user can star changes and see all of them in a view, and watch projects and see their open changes. More advanced users can query by project or status, and even more advanced users can bookmark those queries or even install dashboards into gerrit. We have different types of users, who will want different things from gerrit. I don't think we want the default view to match some sort of expert view, but we should make it easier for users to do more with the tool as they gain more experience. It also doesn't help composability. What if I want to find active patches with less than 100 lines of code change? I have to copy the whole query string to append my delta:100. Copying the query string everywhere makes it easy for inconsistency to slip in. If the guideline changes, will every reviewer update their dashboards and bookmarks? I don't think every reviewer needs to be looking at the same view, so I don't think maintaining consistency is a problem. To share that Oslo dashboard, we have a link in the wiki and we keep it up to date by managing the dashboard creator input file. It doesn't change often (usually only when we start graduating a new library) so it's not a lot of work. Projects have demonstrated a desire to set policy on patch activity centrally, individual custom dashboards don't allow this. Official We should be focusing on maximizing the ability of reviewers to find the patches they do want to review, rather than ensuring that everyone is reviewing the same thing. So far I have only seen discussion of abandoning patches that wouldn't merge anyway. Reviewers who come across those patches will see the -1 from jenkins or the -2 from a core reviewer and either understand that means they should skip reviewing it or waste a small amount of their time until they do learn that. Either way reviewers who want to ignore the patch by using a filter query will be able to continue to do that. custom dashboards (that live on the Gerrit server) are a pain to change AFAIK and we can't count on new contributors knowing how important they are. Allowing projects to automatically flag patches helps both those who read their Gerrit email and those who rely on custom dashboards. Alexis -- Nova Engineer, HP Cloud. AKA lealexis, lxsli. __ 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 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
Doug Hellmann said on Wed, Mar 04, 2015 at 11:10:31AM -0500: I used to use email to track such things, but I have reached the point where keeping up with the push notifications from gerrit would consume all of my waking time. Jim said if his patch was auto-abandoned, he would not find out. There are mail filtering tools, custom dashboards, the REST API. There are a myriad of ways to find out, it seems false to complain about a lack of notification if you turned them off yourself and choose not to use alternative tooling. I'm not saying it's perfect. Gerrit could offer granular control of push notifications. It's also awkward to filter auto-abandoned patches from manually abandoned, which is why I think a new flag or two with bespoke semantics is the best solution. As Jim and others have pointed out, we can identify those changesets using existing attributes rather than having to add a new flag. This doesn't help new contributors who aren't using your custom dashboard. The defaults have to be sensible. The default dashboard must identify patches which will never be reviewed and a push notification should be available for when a patch enters that state. It also doesn't help composability. What if I want to find active patches with less than 100 lines of code change? I have to copy the whole query string to append my delta:100. Copying the query string everywhere makes it easy for inconsistency to slip in. If the guideline changes, will every reviewer update their dashboards and bookmarks? Projects have demonstrated a desire to set policy on patch activity centrally, individual custom dashboards don't allow this. Official custom dashboards (that live on the Gerrit server) are a pain to change AFAIK and we can't count on new contributors knowing how important they are. Allowing projects to automatically flag patches helps both those who read their Gerrit email and those who rely on custom dashboards. Alexis -- Nova Engineer, HP Cloud. AKA lealexis, lxsli. __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
John Griffith john.griffi...@gmail.com writes: Should we just rename this thread to Sensitivity training for contributors? Culture plays a role in shaping ones expectations here. I was lucky enough to grow up in open source culture, so I can identify an automated response easily and I don't take it too seriously. Not everyone has the same culture and although I agree we need to confront these gaps when they impede us, it's more constructive to reach out and bridge the gap than blame the outsider. James E. Blair said on Tue, Mar 03, 2015 at 07:49:03AM -0800: If I had to deprioritize something I was working on and it was auto-abandoned, I would not find out. You should receive messages from Gerrit about this. If you've made the choice to disable or delete those messages, you should expect not to be notified. The review dropping out of your personal dashboard active review queue is a problem though - an email can be forgotten. For what little it's worth, I think having a community-wide definition of inactive and flagging that in the system makes sense. This helps us maintain a clear and consistent policy in our tools. However, I've come around to see that abandon semantics don't fit that flag. We need to narrow in on what inactive really means and what effects the flag should have. We may need two flags, one for 'needs submitter attention' and one for 'needs review attention'. Alexis -- Nova Engineer, HP Cloud. AKA lealexis, lxsli. __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
From: John Griffith [mailto:john.griffi...@gmail.com] Sent: 03 March 2015 14:46 To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics]) On Tue, Mar 3, 2015 at 7:18 AM, Kuvaja, Erno kuv...@hp.commailto:kuv...@hp.com wrote: -Original Message- From: Thierry Carrez [mailto:thie...@openstack.orgmailto:thie...@openstack.org] Sent: 03 March 2015 10:00 To: openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org Subject: Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics]) Doug Wiegley wrote: [...] But I think some of the push back in this thread is challenging this notion that abandoning is negative, which you seem to be treating as a given. I don't. At all. And I don't think I'm alone. I was initially on your side: the abandoned patches are not really deleted, you can easily restore them. So abandoned could just mean inactive or stale in our workflow, and people who actually care can easily unabandon them to make them active again. And since abandoning is currently the only way to permanently get rid of stale / -2ed / undesirable changes anyway, so we should just use that. But words matter, especially for new contributors. For those contributors, someone else abandoning a proposed patch of theirs is a pretty strong move. To them, abandoning should be their decision, not yours (reviewers can -2 patches). Launchpad used to have a similar struggle between real meaning and workflow meaning. It used to have a single status for rejected bugs (Invalid). In the regular bug workflow, that status would be used for valid bugs that you just don't want to fix. But then that created confusion to people outside that workflow since the wrong word was used. So WontFix was introduced as a similar closed state (and then they added Opinion because WontFix seemed too harsh, but that's another story). We have (like always) tension around the precise words we use. You say Abandon is generally used in our community to set inactive. Jim says Abandon should mean abandon and therefore should probably be left to the proposer, and other ways should be used to set inactive. There are multiple solutions to this naming issue. You can rename abandon so that it actually means set inactive or mark as stale. Or you can restrict abandon to the owner of a change, stop defaulting to is:open to list changes, and introduce features in Gerrit so that a is:active query would give you the right thing. But that query would need to be the Gerrit default, not some obscure query you can run or add to your dashboard -- otherwise we are back at step 1. -- Thierry Carrez (ttx) I'd like to ask few questions regarding this as I'm very much pro cleaning the review queues of abandoned stuff. How often people (committer/owner/_reviewer_) abandon changes actively? Now I do not mean the reviewer here only cores marking other peoples abandoned PSs as abandoned I mean how many times you have seen person stating that (s)he will not review a change anymore? I haven't seen that, but I've seen lots of changes where person has reviewed it on some early stage and 10 revisions later still not given ones input again. What I'm trying to say here is that it does not make the change any less abandoned if it's not marked abandoned by the owner. It's rarely active process. Regarding the contributor experience, I'd say it's way more harmful not to mark abandoned changes abandoned than do so. If the person really don't know and can't figure out how to a) join the mailing list b) get to irc c) write a comment to the change or d) reach out anyone in the project in any other means to express that (s)he does not know how to fix the issue flagged in weeks, I'm not sure if we will miss that person as a contributor so much either? And yes, the message should be strong telling that the change has passed the point it most probably will have no traction anymore and active action needs to be taken to continue the workflow. Same time lets turn this around. How many new contributors we drive away because of the reaction Whoa, this many changes have been sitting here for weeks, I have no chance to get my change quickly in? Specifically to Nova, Swift Cinder folks: How much do you see benefit on bug lifecycle management with the abandoning? I would assume bugs that has message their proposed fix abandoned getting way more traction than the ones where the fix has been stale in queue for weeks. And how many of those abandoned ones gets reactivated? Last I'd like to point out that life is full of disappointments. We should not try to keep our community in bubble where no-one ever gets disappointed nor their feelings never gets hurt. I do not appreciate
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Tue, Mar 3, 2015 at 8:46 AM, John Griffith john.griffi...@gmail.com wrote: On Tue, Mar 3, 2015 at 7:18 AM, Kuvaja, Erno kuv...@hp.com wrote: -Original Message- From: Thierry Carrez [mailto:thie...@openstack.org] Sent: 03 March 2015 10:00 To: openstack-dev@lists.openstack.org Subject: Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics]) Doug Wiegley wrote: [...] But I think some of the push back in this thread is challenging this notion that abandoning is negative, which you seem to be treating as a given. I don't. At all. And I don't think I'm alone. I was initially on your side: the abandoned patches are not really deleted, you can easily restore them. So abandoned could just mean inactive or stale in our workflow, and people who actually care can easily unabandon them to make them active again. And since abandoning is currently the only way to permanently get rid of stale / -2ed / undesirable changes anyway, so we should just use that. But words matter, especially for new contributors. For those contributors, someone else abandoning a proposed patch of theirs is a pretty strong move. To them, abandoning should be their decision, not yours (reviewers can -2 patches). Launchpad used to have a similar struggle between real meaning and workflow meaning. It used to have a single status for rejected bugs (Invalid). In the regular bug workflow, that status would be used for valid bugs that you just don't want to fix. But then that created confusion to people outside that workflow since the wrong word was used. So WontFix was introduced as a similar closed state (and then they added Opinion because WontFix seemed too harsh, but that's another story). We have (like always) tension around the precise words we use. You say Abandon is generally used in our community to set inactive. Jim says Abandon should mean abandon and therefore should probably be left to the proposer, and other ways should be used to set inactive. There are multiple solutions to this naming issue. You can rename abandon so that it actually means set inactive or mark as stale. Or you can restrict abandon to the owner of a change, stop defaulting to is:open to list changes, and introduce features in Gerrit so that a is:active query would give you the right thing. But that query would need to be the Gerrit default, not some obscure query you can run or add to your dashboard -- otherwise we are back at step 1. -- Thierry Carrez (ttx) I'd like to ask few questions regarding this as I'm very much pro cleaning the review queues of abandoned stuff. How often people (committer/owner/_reviewer_) abandon changes actively? Now I do not mean the reviewer here only cores marking other peoples abandoned PSs as abandoned I mean how many times you have seen person stating that (s)he will not review a change anymore? I haven't seen that, but I've seen lots of changes where person has reviewed it on some early stage and 10 revisions later still not given ones input again. What I'm trying to say here is that it does not make the change any less abandoned if it's not marked abandoned by the owner. It's rarely active process. Regarding the contributor experience, I'd say it's way more harmful not to mark abandoned changes abandoned than do so. If the person really don't know and can't figure out how to a) join the mailing list b) get to irc c) write a comment to the change or d) reach out anyone in the project in any other means to express that (s)he does not know how to fix the issue flagged in weeks, I'm not sure if we will miss that person as a contributor so much either? And yes, the message should be strong telling that the change has passed the point it most probably will have no traction anymore and active action needs to be taken to continue the workflow. Same time lets turn this around. How many new contributors we drive away because of the reaction Whoa, this many changes have been sitting here for weeks, I have no chance to get my change quickly in? Specifically to Nova, Swift Cinder folks: How much do you see benefit on bug lifecycle management with the abandoning? I would assume bugs that has message their proposed fix abandoned getting way more traction than the ones where the fix has been stale in queue for weeks. And how many of those abandoned ones gets reactivated? Last I'd like to point out that life is full of disappointments. We should not try to keep our community in bubble where no-one ever gets disappointed nor their feelings never gets hurt. I do not appreciate that approach on current trend of raising children and I definitely do not appreciate that approach towards adults. Perhaps the people with bad experience will learn something and get over it or move on. Neither is bad
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
John Griffith john.griffi...@gmail.com writes: Should we just rename this thread to Sensitivity training for contributors? I do not think that only new contributors might feel it is negative. I think that both some new and long-time contributors do. My oldest patch is from July -- it's still relevant, just not important. I just picked up and finished a series of four patches that Jay Pipes left sitting for a month without updates -- but they are really good and should be merged. Yesterday, Monty went through and either cleaned up or abandoned his patches that had been sitting for several months. None of these things offend me or anyone else involved, and they are all enabled by the fact that those changes were not abandoned. And none of these people are new contributors (the opposite, in fact). If I had to deprioritize something I was working on and it was auto-abandoned, I would not find out. It would simply drop from my personal list of patches I am working on, and I would never think about it again. Until, one day perhaps, I see something and think hey, I thought I fixed that. And after a long time digging, I find a patch that someone else abandoned for me. I would be furious that they had made the decision on my behalf that I would do no more work on that. Not everyone's workflow is like this. Not everyone feels the same way about the word abandon. But we have a diversity of contributors in the community and we have a diversity of workflows. We need to help core reviewers focus on patches that are useful to them -- no doubt about it. We need to let new contributors know that we expect them to engage. We have better tools to do both than abandoning, which has negative impacts beyond the immediate purpose of its use. This thread has uncovered some information about how people use Gerrit and where we need to put effort to make sure patches are prioritized correctly. That is really useful information, and I hope other folks will chime in so we can make sure we cover all the bases. -Jim __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
-Original Message- From: Thierry Carrez [mailto:thie...@openstack.org] Sent: 03 March 2015 10:00 To: openstack-dev@lists.openstack.org Subject: Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics]) Doug Wiegley wrote: [...] But I think some of the push back in this thread is challenging this notion that abandoning is negative, which you seem to be treating as a given. I don't. At all. And I don't think I'm alone. I was initially on your side: the abandoned patches are not really deleted, you can easily restore them. So abandoned could just mean inactive or stale in our workflow, and people who actually care can easily unabandon them to make them active again. And since abandoning is currently the only way to permanently get rid of stale / -2ed / undesirable changes anyway, so we should just use that. But words matter, especially for new contributors. For those contributors, someone else abandoning a proposed patch of theirs is a pretty strong move. To them, abandoning should be their decision, not yours (reviewers can -2 patches). Launchpad used to have a similar struggle between real meaning and workflow meaning. It used to have a single status for rejected bugs (Invalid). In the regular bug workflow, that status would be used for valid bugs that you just don't want to fix. But then that created confusion to people outside that workflow since the wrong word was used. So WontFix was introduced as a similar closed state (and then they added Opinion because WontFix seemed too harsh, but that's another story). We have (like always) tension around the precise words we use. You say Abandon is generally used in our community to set inactive. Jim says Abandon should mean abandon and therefore should probably be left to the proposer, and other ways should be used to set inactive. There are multiple solutions to this naming issue. You can rename abandon so that it actually means set inactive or mark as stale. Or you can restrict abandon to the owner of a change, stop defaulting to is:open to list changes, and introduce features in Gerrit so that a is:active query would give you the right thing. But that query would need to be the Gerrit default, not some obscure query you can run or add to your dashboard -- otherwise we are back at step 1. -- Thierry Carrez (ttx) I'd like to ask few questions regarding this as I'm very much pro cleaning the review queues of abandoned stuff. How often people (committer/owner/_reviewer_) abandon changes actively? Now I do not mean the reviewer here only cores marking other peoples abandoned PSs as abandoned I mean how many times you have seen person stating that (s)he will not review a change anymore? I haven't seen that, but I've seen lots of changes where person has reviewed it on some early stage and 10 revisions later still not given ones input again. What I'm trying to say here is that it does not make the change any less abandoned if it's not marked abandoned by the owner. It's rarely active process. Regarding the contributor experience, I'd say it's way more harmful not to mark abandoned changes abandoned than do so. If the person really don't know and can't figure out how to a) join the mailing list b) get to irc c) write a comment to the change or d) reach out anyone in the project in any other means to express that (s)he does not know how to fix the issue flagged in weeks, I'm not sure if we will miss that person as a contributor so much either? And yes, the message should be strong telling that the change has passed the point it most probably will have no traction anymore and active action needs to be taken to continue the workflow. Same time lets turn this around. How many new contributors we drive away because of the reaction Whoa, this many changes have been sitting here for weeks, I have no chance to get my change quickly in? Specifically to Nova, Swift Cinder folks: How much do you see benefit on bug lifecycle management with the abandoning? I would assume bugs that has message their proposed fix abandoned getting way more traction than the ones where the fix has been stale in queue for weeks. And how many of those abandoned ones gets reactivated? Last I'd like to point out that life is full of disappointments. We should not try to keep our community in bubble where no-one ever gets disappointed nor their feelings never gets hurt. I do not appreciate that approach on current trend of raising children and I definitely do not appreciate that approach towards adults. Perhaps the people with bad experience will learn something and get over it or move on. Neither is bad for the community. - Erno __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Mon, Mar 2, 2015 at 8:07 AM, Duncan Thomas duncan.tho...@gmail.com wrote: Why do you say auto-abandon is the wrong tool? I've no problem with the 1 week warning if somebody wants to implement it - I can see the value. A change-set that has been ignored for X weeks is pretty much the dictionary definition of abandoned +1 this I think Tom's suggested help us help you is a great pre-abandon warning. In swift as often as not the last message ended with something like you can catch me on freenode in #openstack-swift if you have any questions But I really can't fathom what's the harm in closing abandoned patches as abandoned? If the author doesn't care about the change enough to address the review comments (or failing tests!) and the core reviewers don't care about it enough to *fix it for them* - where do we think the change is going to go?! It sounds like the argument is just that instead of using abandoned as an explicit description of an implicit state we can just filter these out of every view we use to look for something useful as no changes for X weeks after negative feedback rather than calling a spade a spade. I *mostly* look at patches that don't have feedback. notmyname maintains the swift review dashboard AFAIK: http://goo.gl/r2mxbe It's possible that a pile of abandonded-changes-not-marked-as-abandonded wouldn't actually interrupt my work-flow. But I would imagine maintaining the review dashboard might occasionally require looking at ALL the changes in the queue in an effort to look for a class of changes that aren't getting adequate feedback - that workflow might find the extra noise less than helpful. -Clay __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
Excerpts from Doug Wiegley's message of 2015-03-02 12:47:14 -0800: On Mar 2, 2015, at 1:13 PM, James E. Blair cor...@inaugust.com wrote: Stefano branched this thread from an older one to talk about auto-abandon. In the previous thread, I believe I explained my concerns, but since the topic split, perhaps it would be good to summarize why this is an issue. 1) A core reviewer forcefully abandoning a change contributed by someone else can be a very negative action. It's one thing for a contributor to say I have abandoned this effort, it's very different for a core reviewer to do that for them. It is a very strong action and signal, and should not be taken lightly. I'm not arguing against better tooling, queries, or additional comment warnings. All of those are good things. But I think some of the push back in this thread is challenging this notion that abandoning is negative, which you seem to be treating as a given. I don't. At all. And I don't think I'm alone. I also don't understand your point that the review becomes invisible, since it's a simple gerrit query to see closed reviews, and your own contention is that gerrit queries solve this in the other direction, so it can't be too hard in this one, either. I've done that many times to find mine and others abandoned reviews, the most recent example being resurrecting all of the lbaas v2 reviews after it slipped out of juno and eventually was put into it's own repo. Some of those reviews were abandoned, others not, and it was roughly equivalent to find them, open or not, and then re-tool those for the latest changes to master. You are correct in saying that just like users can query for a proper queue of things they should look at, people can also query for abandoned patches. However, I'm not sure these are actually the same things. One is a simple query to hide things you don't want. The other is a simple query to find things you don't know are missing. __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Mon, 2015-03-02 at 13:35 -0800, Clay Gerrard wrote: I think Tom's suggested help us help you is a great pre-abandon warning. In swift as often as not the last message ended with something like you can catch me on freenode in #openstack-swift if you have any questions Good, this thread is starting to converge. But I really can't fathom what's the harm in closing abandoned patches as abandoned? Jim Blair gave a lot of good reasons for not abandoning *automatically* and instead leave the decision to abandon to humans only. His message is worth reading again: http://lists.openstack.org/pipermail/openstack-dev/2015-March/058104.html /stef __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On 03/03/15 05:35, Clay Gerrard wrote: On Mon, Mar 2, 2015 at 8:07 AM, Duncan Thomas duncan.tho...@gmail.com mailto:duncan.tho...@gmail.com wrote: Why do you say auto-abandon is the wrong tool? I've no problem with the 1 week warning if somebody wants to implement it - I can see the value. A change-set that has been ignored for X weeks is pretty much the dictionary definition of abandoned +1 this I think Tom's suggested help us help you is a great pre-abandon warning. In swift as often as not the last message ended with something like you can catch me on freenode in #openstack-swift if you have any questions But I really can't fathom what's the harm in closing abandoned patches as abandoned? It might be an interesting exercise to consider how areas like feedback, criticism or asking for help could potentially differ in cultures and levels of skill other than the one with which one may be most familiar. Now, look at the wording of my above sentence and consider whether you'd ever write it that way. Pretty damn indirect, and vague right? It turns out that there are large swathes of the world that operate in this much more nuanced way. Taking direct action against something someone has produced using (from their perspective) strong/emotive language can be at basically the same level as punching someone in the face and yelling You suck! in other areas :) I'm sure you are aware of these things - I don't mean to preach, but I thought it would be a good chance to explain what what the help us help you message might come across to these kind of folks: * This isn't your fault, it's OK! * We're here to help, and you have permission to ask us for help. * Here are some steps you can take, and you have permission to take those steps. * Here are some standard procedures that everyone follows, so if you follow them you won't be caught standing out. * If something happens after this, it's a random third party actor that's doing it (the system), not a person criticising you. Anyway, I guess I better dig up jeepyb again ... If the author doesn't care about the change enough to address the review comments (or failing tests!) and the core reviewers don't care about it enough to *fix it for them* - where do we think the change is going to go?! It sounds like the argument is just that instead of using abandoned as an explicit description of an implicit state we can just filter these out of every view we use to look for something useful as no changes for X weeks after negative feedback rather than calling a spade a spade. I *mostly* look at patches that don't have feedback. notmyname maintains the swift review dashboard AFAIK: http://goo.gl/r2mxbe It's possible that a pile of abandonded-changes-not-marked-as-abandonded wouldn't actually interrupt my work-flow. But I would imagine maintaining the review dashboard might occasionally require looking at ALL the changes in the queue in an effort to look for a class of changes that aren't getting adequate feedback - that workflow might find the extra noise less than helpful. -Clay __ 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 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Mar 2, 2015, at 1:13 PM, James E. Blair cor...@inaugust.com wrote: Stefano branched this thread from an older one to talk about auto-abandon. In the previous thread, I believe I explained my concerns, but since the topic split, perhaps it would be good to summarize why this is an issue. 1) A core reviewer forcefully abandoning a change contributed by someone else can be a very negative action. It's one thing for a contributor to say I have abandoned this effort, it's very different for a core reviewer to do that for them. It is a very strong action and signal, and should not be taken lightly. I'm not arguing against better tooling, queries, or additional comment warnings. All of those are good things. But I think some of the push back in this thread is challenging this notion that abandoning is negative, which you seem to be treating as a given. I don't. At all. And I don't think I'm alone. I also don't understand your point that the review becomes invisible, since it's a simple gerrit query to see closed reviews, and your own contention is that gerrit queries solve this in the other direction, so it can't be too hard in this one, either. I've done that many times to find mine and others abandoned reviews, the most recent example being resurrecting all of the lbaas v2 reviews after it slipped out of juno and eventually was put into it's own repo. Some of those reviews were abandoned, others not, and it was roughly equivalent to find them, open or not, and then re-tool those for the latest changes to master. Back to your question of what queries are most useful, I already answered, but to give you an idea of how we directed folks to find reviews relevant to kilo-2, I'll share this monster, which didn't even include targeted bugs we wanted looked at. Some tighter integration with launchpad (or storyboard) would likely be necessary for this to be sane. Neutron, kilo-2 blueprints: https://review.openstack.org/#/q/project:openstack/neutron+status:open+(topic:bp/wsgi-pecan-switch+OR+topic:bp/plugin-interface-perestroika+OR+topic:bp/reorganize-unit-test-tree+OR+topic:bp/restructure-l2-agent+OR+topic:bp/rootwrap-daemon-mode+OR+topic:bp/retargetable-functional-testing+OR+topic:bp/refactor-iptables-firewall-driver+OR+topic:bp/vsctl-to-ovsdb+OR+topic:bp/lbaas-api-and-objmodel-improvement+OR+topic:bp/restructure-l3-agent+OR+topic:bp/neutron-ovs-dvr-vlan+OR+topic:bp/allow-specific-floating-ip-address+OR+topic:bp/ipset-manager-refactor+OR+topic:bp/agent-child-processes-status+OR+topic:bp/extra-dhcp-opts-ipv4-ipv6+OR+topic:bp/ipsec-strongswan-driver+OR+topic:bp/ofagent-bridge-setup+OR+topic:bp/arp-spoof-patch-ebtables+OR+topic:bp/report-ha-router-master+OR+topic:bp/conntrack-in-security-group+OR+topic:bp/allow-mac-to-be-updated+OR+topic:bp/specify-router-ext-ip+OR+topic:bp/a10-lbaas-v2-driver+OR+topic:bp/brocade-vyatta-fwaas-plugin+OR+topic:bp/netscaler-lbaas-v2-driver+O R+topic:bp/ofagent-sub-driver+OR+topic:bp/ml2-cisco-nexus-mechdriver-providernet+OR+topic:bp/ofagent-flow-based-tunneling+OR+topic:bp/ml2-ovs-portsecurity+OR+topic:bp/fwaas-cisco+OR+topic:bp/freescale-fwaas-plugin+OR+topic:bp/rpc-docs-and-namespace),n,z Thanks, doug 2) Many changes become inactive due to no fault of their authors. For instance, a change to nova that missed a freeze deadline might need to be deferred for 3 months or more. It should not be automatically abandoned. 3) Abandoned changes are not visible by their authors. Many contributors will not see the abandoned change. Many contributors use their list of open reviews to get their work done, but if you abandon their changes, they will no longer see that there is work for them to be done. 4) Abandoned changes are not visible to other contributors. Other people contributing to a project may see a change that they could fix up and get merged. However, if the change is abandoned, they are unlikely to find it. 5) Abandoned changes are not able to be resumed by other contributors. Even if they managed to find changes despite the obstacles imposed by #3, they would be unable to restore the change and continue working on it. In short, there are a number of negative impacts to contributors, core reviewers, and maintainers of projects caused by automatically abandoning changes. These are not hypothetical; I have seen all of these negative impacts on projects I contribute to. Now this is the most important part -- I can not emphasize this enough: Whatever is being achieved by auto-abandoning can be achieved through other, less harmful, methods. Core reviewers should not have to wade through lots of extra changes. They should not be called upon to deal with drive-by changes that people are not willing to collaborate on. Abandoning changes is an imperfect solution to a problem, and we can find a better solution. We have tools that can filter out changes that are not active so that core
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
Duncan Thomas duncan.tho...@gmail.com writes: Why do you say auto-abandon is the wrong tool? I've no problem with the 1 week warning if somebody wants to implement it - I can see the value. A change-set that has been ignored for X weeks is pretty much the dictionary definition of abandoned, and restoring it is one mouse click. Maybe put something more verbose in the auto-abandon message than we have been, encouraging those who feel it shouldn't have been marked abandoned to restore it (and respond quicker in future) but other than that we seem to be using the right tool to my eyes Why do you feel the need to abandon changes submitted by other people? Is it because you have a list of changes to review, and they persist on that list? If so, let's work on making a better list for you. We have the tools. What query/page/list/etc are you looking at where you see changes that you don't want to see? -Jim __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Mar 2, 2015, at 11:44 AM, James E. Blair cor...@inaugust.com wrote: Duncan Thomas duncan.tho...@gmail.com writes: Why do you say auto-abandon is the wrong tool? I've no problem with the 1 week warning if somebody wants to implement it - I can see the value. A change-set that has been ignored for X weeks is pretty much the dictionary definition of abandoned, and restoring it is one mouse click. Maybe put something more verbose in the auto-abandon message than we have been, encouraging those who feel it shouldn't have been marked abandoned to restore it (and respond quicker in future) but other than that we seem to be using the right tool to my eyes Why do you feel the need to abandon changes submitted by other people? Why do you feel the need to keep them? Do your regularly look at older patches? Do you know anyone that does? Speaking as a contributor, I personally vastly prefer clicking 'Restore' to having gerrit being a haystack of cruft. I have/had many frustrations trying to become a useful contributor. Abandoned patches was never one of them. Is it because you have a list of changes to review, and they persist on that list? If so, let's work on making a better list for you. We have the tools. What query/page/list/etc are you looking at where you see changes that you don't want to see? When I was starting out, hearing that the best thing I could help out was to do some reviews, I'd naively browse to gerrit and look for something easy to get started with. The default query (status:open) means that there is about a 110% (hyperbole added) chance that I'll pick something to review that's a waste of time. A default query that edited out old, jenkins failing, and -2 stuff would be helpful. A default or easy query that highlighted things relevant to the current milestone's blueprints and bugs would be SUPER useful to guiding folks towards the most useful reviews to be doing for a given project. The current system does not do a good job of intuitively guiding folks towards the right answer. You have to know the tribal knowledge first, and/or which of six conflicting wiki pages has the right info to get started with (more hyperbole added.) Thanks, doug -Jim __ 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 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
Doug Wiegley doug...@parksidesoftware.com writes: A default query that edited out old, jenkins failing, and -2 stuff would be helpful. A default or easy query that highlighted things relevant to the current milestone's blueprints and bugs would be SUPER useful to guiding folks towards the most useful reviews to be doing for a given project. Great, this is the kind of information I'm looking for. Please tell me what page or query you specifically use to find reviews, and let's work to make sure it shows you the right information. -Jim __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
Stefano branched this thread from an older one to talk about auto-abandon. In the previous thread, I believe I explained my concerns, but since the topic split, perhaps it would be good to summarize why this is an issue. 1) A core reviewer forcefully abandoning a change contributed by someone else can be a very negative action. It's one thing for a contributor to say I have abandoned this effort, it's very different for a core reviewer to do that for them. It is a very strong action and signal, and should not be taken lightly. 2) Many changes become inactive due to no fault of their authors. For instance, a change to nova that missed a freeze deadline might need to be deferred for 3 months or more. It should not be automatically abandoned. 3) Abandoned changes are not visible by their authors. Many contributors will not see the abandoned change. Many contributors use their list of open reviews to get their work done, but if you abandon their changes, they will no longer see that there is work for them to be done. 4) Abandoned changes are not visible to other contributors. Other people contributing to a project may see a change that they could fix up and get merged. However, if the change is abandoned, they are unlikely to find it. 5) Abandoned changes are not able to be resumed by other contributors. Even if they managed to find changes despite the obstacles imposed by #3, they would be unable to restore the change and continue working on it. In short, there are a number of negative impacts to contributors, core reviewers, and maintainers of projects caused by automatically abandoning changes. These are not hypothetical; I have seen all of these negative impacts on projects I contribute to. Now this is the most important part -- I can not emphasize this enough: Whatever is being achieved by auto-abandoning can be achieved through other, less harmful, methods. Core reviewers should not have to wade through lots of extra changes. They should not be called upon to deal with drive-by changes that people are not willing to collaborate on. Abandoning changes is an imperfect solution to a problem, and we can find a better solution. We have tools that can filter out changes that are not active so that core reviewers are not bothered by them. In fact, the auto-abandon script itself is built on one or two exceedingly simple queries which, when reversed, will show you only the changes it would not abandon. What I hope to gain by this conversation is to identify where the gaps in our tooling are. If you feel strongly that you do not want to see inactive changes, please tell me what query, dashboard, tool, page, etc., that you use to find changes to review. We can help make sure that it is structured to filter out changes you are not interested in, and helps surface changes you want to work on. Thanks, Jim __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Mon, Mar 2, 2015 at 12:52 PM, James E. Blair cor...@inaugust.com wrote: Doug Wiegley doug...@parksidesoftware.com writes: A default query that edited out old, jenkins failing, and -2 stuff would be helpful. A default or easy query that highlighted things relevant to the current milestone's blueprints and bugs would be SUPER useful to guiding folks towards the most useful reviews to be doing for a given project. Great, this is the kind of information I'm looking for. Please tell me what page or query you specifically use to find reviews, and let's work to make sure it shows you the right information. -Jim __ 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 Wow.. so some really good (and some surprising) viewpoints. There are things I hadn't thought about it here so that's great. I also like the idea from Tom of an automatic comment being added, that's pretty cool in my opinion. I also think James makes some great points about the query tools, combining all of this sounds great. Thinking more about this today and the reality is, it's been several months since I've made it far enough down my review list in gerrit to get to anything more than a few days old anyway, so I suppose if it presents a better experience for the submitter, that's great. In other words it probably doesn't matter any more if it's in the queue or not. My own personal opinions (stated earlier) aside. I think compromise along the lines suggested is a great approach and a win for everybody involved. __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
+2 I recently had a patch abandoned that had every right to be pulled off the list. It had fallen off my radar and was no longer relevant. As long as there is a way to restore the patch where appropriate, we should do it. Jay I have to agree with John. We have many more submitters than we have core folks, and our current scaling limits tend to be around core and reviews, not around submissions, so making life slightly more difficult for submitters in order to make it substantially easier for core is a reasonable trade. Not marking a review as abandoned if feedback hasn't been responded to in 2 week misleads reviewers and submitters, so I fully support the cinder abandonment rules - the 'restore change' button takes a moment to click. On 28 February 2015 at 03:02, John Griffith john.griffi...@gmail.com wrote: On Fri, Feb 27, 2015 at 5:40 PM, Stefano Maffulli stef...@openstack.org wrote: I'm not expressing myself cleary enough. I don't advocate for the removal of anything because I like pretty charts. I'm changing the subject to be even more clear. On Fri, 2015-02-27 at 13:26 -0800, James E. Blair wrote: I am asking you to please independently remove changes that you don't think should be considered from your metrics. I'm saying that the reports have indicators that seem to show struggle. In a previous message Kevin hinted that probably a reason for those bad looking numbers was due to a lot of reviews that appear to have been abandoned. This doesn't seem the case because some projects have a habit of 'purging'. I have never explicitly ordered developers to purge anything. If their decision to purge is due to the numbers they may have seen on the reports I'd like to know. That said, the problem that the reports highlight remains confirmed so far: contributors seem to be left in some cases hanging, for many many days, *after a comment* and they don't come back. I think abandoning changes so that the metrics look the way we want is a terrible experience for contributors. I agree, that should not be a motivator. Also, after chatting with you on IRC I tend to think that instead of abandoning the review we should highlight them and have humans act on them. Maybe build a dashboard of 'old stuff' to periodically sift through and see if there are things worth picking up again or to ping the owner or something else managed by humans. I happened to have found one of such review automatically closed that could have been fixed with a trivial edit in commit message instead: https://review.openstack.org/#/c/98735/ (that owner had a bunch of auto-abandoned patches https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08%2540gmail.com%253E%22,n,z). I have made a note to reach out to him and get more anecdotes. Especially as it appears some projects, such as nova, are in a position where they are actually leaving -2 votes on changes which will not be lifted for 2 or 3 months. That means that if someone runs a script like Sean's, these changes will be abandoned, yet there is nothing that the submitter can do to progress the change in the mean time. Abandoning such a review is making an already bad experience for contributors even worse. this sounds like a different issue :( __ 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 For what it's worth, at one point the Cinder project setup an auto-abandon job that did purge items that had a negative mark either from a reviewer or from Jenkins and had not been updated in over two weeks. This had absolutely nothing to do with metrics or statistical analysis of the project. We simply had a hard time dealing with patches that the submitter didn't care about. If somebody takes the time to review a patch, then I don't think it's too much to ask that the submitter respond to questions or comments within a two week period. Note, the auto purge in our case only purged items that had no updates or activity at all. We were actually in a position where we had patches that were submitted, failed unit tests in the gate (valid failures that occurred 100% of the time) and had sat for an entire release without the submitter ever updating the patch. I don't think it's unreasonable at all to abandon these and remove them from the queue. I don't think this is a bad thing, I think it's worse to leave them as active when they're bit-rotted and the submitter doesn't even care about them any longer. The other thing is, those patches are still there, they can still be accessed and reinstated. There's a lot of knocks against core teams regarding time to review and keeping
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On 28/02/15 09:02, John Griffith wrote: On Fri, Feb 27, 2015 at 5:40 PM, Stefano Maffulli stef...@openstack.org mailto:stef...@openstack.org wrote: I'm not expressing myself cleary enough. I don't advocate for the removal of anything because I like pretty charts. I'm changing the subject to be even more clear. On Fri, 2015-02-27 at 13:26 -0800, James E. Blair wrote: I am asking you to please independently remove changes that you don't think should be considered from your metrics. I'm saying that the reports have indicators that seem to show struggle. In a previous message Kevin hinted that probably a reason for those bad looking numbers was due to a lot of reviews that appear to have been abandoned. This doesn't seem the case because some projects have a habit of 'purging'. I have never explicitly ordered developers to purge anything. If their decision to purge is due to the numbers they may have seen on the reports I'd like to know. That said, the problem that the reports highlight remains confirmed so far: contributors seem to be left in some cases hanging, for many many days, *after a comment* and they don't come back. I think abandoning changes so that the metrics look the way we want is a terrible experience for contributors. I agree, that should not be a motivator. Also, after chatting with you on IRC I tend to think that instead of abandoning the review we should highlight them and have humans act on them. Maybe build a dashboard of 'old stuff' to periodically sift through and see if there are things worth picking up again or to ping the owner or something else managed by humans. I happened to have found one of such review automatically closed that could have been fixed with a trivial edit in commit message instead: https://review.openstack.org/#/c/98735/ (that owner had a bunch of auto-abandoned patches https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z). I have made a note to reach out to him and get more anecdotes. Especially as it appears some projects, such as nova, are in a position where they are actually leaving -2 votes on changes which will not be lifted for 2 or 3 months. That means that if someone runs a script like Sean's, these changes will be abandoned, yet there is nothing that the submitter can do to progress the change in the mean time. Abandoning such a review is making an already bad experience for contributors even worse. this sounds like a different issue :( __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev For what it's worth, at one point the Cinder project setup an auto-abandon job that did purge items that had a negative mark either from a reviewer or from Jenkins and had not been updated in over two weeks. This had absolutely nothing to do with metrics or statistical analysis of the project. We simply had a hard time dealing with patches that the submitter didn't care about. If somebody takes the time to review a patch, then I don't think it's too much to ask that the submitter respond to questions or comments within a two week period. Note, the auto purge in our case only purged items that had no updates or activity at all. We were actually in a position where we had patches that were submitted, failed unit tests in the gate (valid failures that occurred 100% of the time) and had sat for an entire release without the submitter ever updating the patch. I don't think it's unreasonable at all to abandon these and remove them from the queue. I don't think this is a bad thing, I think it's worse to leave them as active when they're bit-rotted and the submitter doesn't even care about them any longer. The other thing is, those patches are still there, they can still be accessed and reinstated. There's a lot of knocks against core teams regarding time to review and keeping up with the work load.. that's fine, but at the same time if you submit something you should follow through on it and respond to comments or test failures in a timely manner. Also there should be some scaling factor here; if a patch that needs updating should be expected to be able to sit in the queue for a month for example, then we should give one month for each reviewer; so minimum of three months for a +1, +2 and +A. I don't think it's
Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
I have to agree with John. We have many more submitters than we have core folks, and our current scaling limits tend to be around core and reviews, not around submissions, so making life slightly more difficult for submitters in order to make it substantially easier for core is a reasonable trade. Not marking a review as abandoned if feedback hasn't been responded to in 2 week misleads reviewers and submitters, so I fully support the cinder abandonment rules - the 'restore change' button takes a moment to click. On 28 February 2015 at 03:02, John Griffith john.griffi...@gmail.com wrote: On Fri, Feb 27, 2015 at 5:40 PM, Stefano Maffulli stef...@openstack.org wrote: I'm not expressing myself cleary enough. I don't advocate for the removal of anything because I like pretty charts. I'm changing the subject to be even more clear. On Fri, 2015-02-27 at 13:26 -0800, James E. Blair wrote: I am asking you to please independently remove changes that you don't think should be considered from your metrics. I'm saying that the reports have indicators that seem to show struggle. In a previous message Kevin hinted that probably a reason for those bad looking numbers was due to a lot of reviews that appear to have been abandoned. This doesn't seem the case because some projects have a habit of 'purging'. I have never explicitly ordered developers to purge anything. If their decision to purge is due to the numbers they may have seen on the reports I'd like to know. That said, the problem that the reports highlight remains confirmed so far: contributors seem to be left in some cases hanging, for many many days, *after a comment* and they don't come back. I think abandoning changes so that the metrics look the way we want is a terrible experience for contributors. I agree, that should not be a motivator. Also, after chatting with you on IRC I tend to think that instead of abandoning the review we should highlight them and have humans act on them. Maybe build a dashboard of 'old stuff' to periodically sift through and see if there are things worth picking up again or to ping the owner or something else managed by humans. I happened to have found one of such review automatically closed that could have been fixed with a trivial edit in commit message instead: https://review.openstack.org/#/c/98735/ (that owner had a bunch of auto-abandoned patches https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08%2540gmail.com%253E%22,n,z). I have made a note to reach out to him and get more anecdotes. Especially as it appears some projects, such as nova, are in a position where they are actually leaving -2 votes on changes which will not be lifted for 2 or 3 months. That means that if someone runs a script like Sean's, these changes will be abandoned, yet there is nothing that the submitter can do to progress the change in the mean time. Abandoning such a review is making an already bad experience for contributors even worse. this sounds like a different issue :( __ 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 For what it's worth, at one point the Cinder project setup an auto-abandon job that did purge items that had a negative mark either from a reviewer or from Jenkins and had not been updated in over two weeks. This had absolutely nothing to do with metrics or statistical analysis of the project. We simply had a hard time dealing with patches that the submitter didn't care about. If somebody takes the time to review a patch, then I don't think it's too much to ask that the submitter respond to questions or comments within a two week period. Note, the auto purge in our case only purged items that had no updates or activity at all. We were actually in a position where we had patches that were submitted, failed unit tests in the gate (valid failures that occurred 100% of the time) and had sat for an entire release without the submitter ever updating the patch. I don't think it's unreasonable at all to abandon these and remove them from the queue. I don't think this is a bad thing, I think it's worse to leave them as active when they're bit-rotted and the submitter doesn't even care about them any longer. The other thing is, those patches are still there, they can still be accessed and reinstated. There's a lot of knocks against core teams regarding time to review and keeping up with the work load.. that's fine, but at the same time if you submit something you should follow through on it and respond to comments or test failures in a timely manner. Also there should be some scaling factor here; if a
[openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
I'm not expressing myself cleary enough. I don't advocate for the removal of anything because I like pretty charts. I'm changing the subject to be even more clear. On Fri, 2015-02-27 at 13:26 -0800, James E. Blair wrote: I am asking you to please independently remove changes that you don't think should be considered from your metrics. I'm saying that the reports have indicators that seem to show struggle. In a previous message Kevin hinted that probably a reason for those bad looking numbers was due to a lot of reviews that appear to have been abandoned. This doesn't seem the case because some projects have a habit of 'purging'. I have never explicitly ordered developers to purge anything. If their decision to purge is due to the numbers they may have seen on the reports I'd like to know. That said, the problem that the reports highlight remains confirmed so far: contributors seem to be left in some cases hanging, for many many days, *after a comment* and they don't come back. I think abandoning changes so that the metrics look the way we want is a terrible experience for contributors. I agree, that should not be a motivator. Also, after chatting with you on IRC I tend to think that instead of abandoning the review we should highlight them and have humans act on them. Maybe build a dashboard of 'old stuff' to periodically sift through and see if there are things worth picking up again or to ping the owner or something else managed by humans. I happened to have found one of such review automatically closed that could have been fixed with a trivial edit in commit message instead: https://review.openstack.org/#/c/98735/ (that owner had a bunch of auto-abandoned patches https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z). I have made a note to reach out to him and get more anecdotes. Especially as it appears some projects, such as nova, are in a position where they are actually leaving -2 votes on changes which will not be lifted for 2 or 3 months. That means that if someone runs a script like Sean's, these changes will be abandoned, yet there is nothing that the submitter can do to progress the change in the mean time. Abandoning such a review is making an already bad experience for contributors even worse. this sounds like a different issue :( __ 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] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])
On Fri, Feb 27, 2015 at 5:40 PM, Stefano Maffulli stef...@openstack.org wrote: I'm not expressing myself cleary enough. I don't advocate for the removal of anything because I like pretty charts. I'm changing the subject to be even more clear. On Fri, 2015-02-27 at 13:26 -0800, James E. Blair wrote: I am asking you to please independently remove changes that you don't think should be considered from your metrics. I'm saying that the reports have indicators that seem to show struggle. In a previous message Kevin hinted that probably a reason for those bad looking numbers was due to a lot of reviews that appear to have been abandoned. This doesn't seem the case because some projects have a habit of 'purging'. I have never explicitly ordered developers to purge anything. If their decision to purge is due to the numbers they may have seen on the reports I'd like to know. That said, the problem that the reports highlight remains confirmed so far: contributors seem to be left in some cases hanging, for many many days, *after a comment* and they don't come back. I think abandoning changes so that the metrics look the way we want is a terrible experience for contributors. I agree, that should not be a motivator. Also, after chatting with you on IRC I tend to think that instead of abandoning the review we should highlight them and have humans act on them. Maybe build a dashboard of 'old stuff' to periodically sift through and see if there are things worth picking up again or to ping the owner or something else managed by humans. I happened to have found one of such review automatically closed that could have been fixed with a trivial edit in commit message instead: https://review.openstack.org/#/c/98735/ (that owner had a bunch of auto-abandoned patches https://review.openstack.org/#/q/owner:%22Mh+Raies+%253Craiesmh08% 2540gmail.com%253E%22,n,z). I have made a note to reach out to him and get more anecdotes. Especially as it appears some projects, such as nova, are in a position where they are actually leaving -2 votes on changes which will not be lifted for 2 or 3 months. That means that if someone runs a script like Sean's, these changes will be abandoned, yet there is nothing that the submitter can do to progress the change in the mean time. Abandoning such a review is making an already bad experience for contributors even worse. this sounds like a different issue :( __ 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 For what it's worth, at one point the Cinder project setup an auto-abandon job that did purge items that had a negative mark either from a reviewer or from Jenkins and had not been updated in over two weeks. This had absolutely nothing to do with metrics or statistical analysis of the project. We simply had a hard time dealing with patches that the submitter didn't care about. If somebody takes the time to review a patch, then I don't think it's too much to ask that the submitter respond to questions or comments within a two week period. Note, the auto purge in our case only purged items that had no updates or activity at all. We were actually in a position where we had patches that were submitted, failed unit tests in the gate (valid failures that occurred 100% of the time) and had sat for an entire release without the submitter ever updating the patch. I don't think it's unreasonable at all to abandon these and remove them from the queue. I don't think this is a bad thing, I think it's worse to leave them as active when they're bit-rotted and the submitter doesn't even care about them any longer. The other thing is, those patches are still there, they can still be accessed and reinstated. There's a lot of knocks against core teams regarding time to review and keeping up with the work load.. that's fine, but at the same time if you submit something you should follow through on it and respond to comments or test failures in a timely manner. Also there should be some scaling factor here; if a patch that needs updating should be expected to be able to sit in the queue for a month for example, then we should give one month for each reviewer; so minimum of three months for a +1, +2 and +A. I don't think it's reasonable to say hey, you all have to review faster and get more done and then also say by the way, you need to babysit and reach out and contact owners of patches that have been idle for long periods. Especially considering MANY of the patches in Cinder at least that end up falling into this category are from folks that aren't on IRC and do not have public email addresses in LaunchPad. Just providing another perspective.