Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])

2015-03-05 Thread Doug Hellmann


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])

2015-03-05 Thread Alexis Lee
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])

2015-03-04 Thread Alexis Lee
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])

2015-03-03 Thread Kuvaja, Erno
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])

2015-03-03 Thread Kyle Mestery
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])

2015-03-03 Thread James E. Blair
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])

2015-03-03 Thread Kuvaja, Erno
 -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])

2015-03-02 Thread Clay Gerrard
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])

2015-03-02 Thread Clint Byrum
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])

2015-03-02 Thread Stefano Maffulli
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])

2015-03-02 Thread Tom Fifield
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])

2015-03-02 Thread Doug Wiegley

 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])

2015-03-02 Thread James E. Blair
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])

2015-03-02 Thread Doug Wiegley

 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])

2015-03-02 Thread James E. Blair
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])

2015-03-02 Thread James E. Blair
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])

2015-03-02 Thread John Griffith
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])

2015-03-01 Thread Jay Bryant
+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])

2015-03-01 Thread Tom Fifield
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])

2015-03-01 Thread Duncan Thomas
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 

Re: [openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])

2015-02-27 Thread John Griffith
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.