Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Sean Dague]

On 12/05/2014 08:05 AM, Joe Gordon wrote:
> 
> On Dec 5, 2014 7:07 AM, "Daniel P. Berrange"  > wrote:
>>
>> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
>> > Nova currently has 197 patches that have seen no activity in the last 4
>> > weeks (project:openstack/nova age:4weeks status:open).
>>
>> On a somewhat related note, nova-specs currently has 17 specs
>> open against specs/juno, most with -2 votes. I think we should
>> just mass-abandon anything still touching the specs/juno directory.
>> If people cared about them still they would have submitted for
>> specs/kilo.
>>
>> So any objection to killing everything in the list below:
> 
> +1, makes sense to me.

Agreed. +1.

> 
>>
>>
> +-+---+--+---+-+--+
>> | URL | Subject   
>| Created  | Tests | Reviews | Workflow |
>>
> +-+---+--+---+-+--+
>> | https://review.openstack.org/86938  | Add tasks to the v3 API   
>| 237 days |  1| -2  |  |
>> | https://review.openstack.org/88334  | Add support for USB
> controller| 231 days |  1| -2  |  |
>> | https://review.openstack.org/89766  | Add useful metrics into
> utilization based scheduli... | 226 days |  1| -2  |  |
>> | https://review.openstack.org/90239  | Blueprint for Cinder Multi
> attach volumes | 224 days |  1| -2  |  |
>> | https://review.openstack.org/90647  | Add utilization based weighers
> on top of MetricsWe... | 221 days |  1| -2  |  |
>> | https://review.openstack.org/96543  | Smart Scheduler (Solver
> Scheduler) - Constraint ba... | 189 days |  1| -2  |  |
>> | https://review.openstack.org/97441  | Add nova spec for
> bp/isnot-operator   | 185 days |  1| -2  | 
> |
>> | https://review.openstack.org/99476  | Dedicate aggregates for
> specific tenants  | 176 days |  1| -2  |  |
>> | https://review.openstack.org/99576  | Add client token to
> CreateServer  | 176 days |  1| -2  |  |
>> | https://review.openstack.org/101921 | Spec for Neutron migration
> feature| 164 days |  1| -2  |  |
>> | https://review.openstack.org/103617 | Support Identity V3 API   
>| 157 days |  1| -1  |  |
>> | https://review.openstack.org/105385 | Leverage the features of IBM
> GPFS to store cached ... | 150 days |  1| -2  |  |
>> | https://review.openstack.org/108582 | Add ironic boot mode filters 
> | 136 days |  1| -2  |  |
>> | https://review.openstack.org/110639 | Blueprint for the
> implementation of Nested Quota D... | 127 days |  1| -2  | 
> |
>> | https://review.openstack.org/111308 | Added VirtProperties object
> blueprint | 125 days |  1| -2  |  |
>> | https://review.openstack.org/111745 | Improve instance boot time   
> | 122 days |  1| -2  |  |
>> | https://review.openstack.org/116280 | Add a new filter to implement
> project isolation fe... | 104 days |  1| -2  |  |
>>
> +-+---+--+---+-+--+
>>
>>
>> Regards,
>> Daniel
>> --
>> |: http://berrange.com  -o-   
> http://www.flickr.com/photos/dberrange/ :|
>> |: http://libvirt.org  -o-   
>  http://virt-manager.org :|
>> |: http://autobuild.org   -o-   
>  http://search.cpan.org/~danberr/ :|
>> |: http://entangle-photo.org   -o- 
>  http://live.gnome.org/gtk-vnc :|
>>
>> ___
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
> 
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


-- 
Sean Dague
http://dague.net

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Joe Gordon]

On Dec 5, 2014 7:07 AM, "Daniel P. Berrange"  wrote:
>
> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> > Nova currently has 197 patches that have seen no activity in the last 4
> > weeks (project:openstack/nova age:4weeks status:open).
>
> On a somewhat related note, nova-specs currently has 17 specs
> open against specs/juno, most with -2 votes. I think we should
> just mass-abandon anything still touching the specs/juno directory.
> If people cared about them still they would have submitted for
> specs/kilo.
>
> So any objection to killing everything in the list below:

+1, makes sense to me.

>
>
+-+---+--+---+-+--+
> | URL | Subject
 | Created  | Tests | Reviews | Workflow |
>
+-+---+--+---+-+--+
> | https://review.openstack.org/86938  | Add tasks to the v3 API
 | 237 days |  1| -2  |  |
> | https://review.openstack.org/88334  | Add support for USB controller
| 231 days |  1| -2  |  |
> | https://review.openstack.org/89766  | Add useful metrics into
utilization based scheduli... | 226 days |  1| -2  |  |
> | https://review.openstack.org/90239  | Blueprint for Cinder Multi attach
volumes | 224 days |  1| -2  |  |
> | https://review.openstack.org/90647  | Add utilization based weighers on
top of MetricsWe... | 221 days |  1| -2  |  |
> | https://review.openstack.org/96543  | Smart Scheduler (Solver
Scheduler) - Constraint ba... | 189 days |  1| -2  |  |
> | https://review.openstack.org/97441  | Add nova spec for
bp/isnot-operator   | 185 days |  1| -2  |
|
> | https://review.openstack.org/99476  | Dedicate aggregates for specific
tenants  | 176 days |  1| -2  |  |
> | https://review.openstack.org/99576  | Add client token to CreateServer
| 176 days |  1| -2  |  |
> | https://review.openstack.org/101921 | Spec for Neutron migration
feature| 164 days |  1| -2  |  |
> | https://review.openstack.org/103617 | Support Identity V3 API
 | 157 days |  1| -1  |  |
> | https://review.openstack.org/105385 | Leverage the features of IBM GPFS
to store cached ... | 150 days |  1| -2  |  |
> | https://review.openstack.org/108582 | Add ironic boot mode filters
| 136 days |  1| -2  |  |
> | https://review.openstack.org/110639 | Blueprint for the implementation
of Nested Quota D... | 127 days |  1| -2  |  |
> | https://review.openstack.org/111308 | Added VirtProperties object
blueprint | 125 days |  1| -2  |  |
> | https://review.openstack.org/111745 | Improve instance boot time
| 122 days |  1| -2  |  |
> | https://review.openstack.org/116280 | Add a new filter to implement
project isolation fe... | 104 days |  1| -2  |  |
>
+-+---+--+---+-+--+
>
>
> Regards,
> Daniel
> --
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/
:|
> |: http://libvirt.org  -o- http://virt-manager.org
:|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/
:|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc
:|
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Daniel P. Berrange]

On Fri, Dec 05, 2014 at 11:16:28PM +1100, Matthew Oliver wrote:
> I have a script that does 95% of what you want:
> 
> https://github.com/matthewoliver/swift_abandon_notifier
> 
> We are using it for swift reviews. At the moment the only thing it doesn't
> do is actually abandon, it instead sends a warning email and waits n days
> (2 weeks by default) for action, if it still turns up it adds it to a list
> of abandoned changes.

Nova already has a similar script that does the abandon too, but both
yours & novas are based on activity / review feedback. I'm explicitly
considering abanadoning based on the file path for the spec.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Matthew Oliver]

I have a script that does 95% of what you want:

https://github.com/matthewoliver/swift_abandon_notifier

We are using it for swift reviews. At the moment the only thing it doesn't
do is actually abandon, it instead sends a warning email and waits n days
(2 weeks by default) for action, if it still turns up it adds it to a list
of abandoned changes.

Eg: http://abandoner.oliver.net.au

So anything that appears in that list can be abandoned by a core.

Feel free to use it (just uses a yaml file for configuration) and we can
all benefit from enhancements made ;)

Matt
On Dec 5, 2014 11:06 PM, "Daniel P. Berrange"  wrote:

> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> > Nova currently has 197 patches that have seen no activity in the last 4
> > weeks (project:openstack/nova age:4weeks status:open).
>
> On a somewhat related note, nova-specs currently has 17 specs
> open against specs/juno, most with -2 votes. I think we should
> just mass-abandon anything still touching the specs/juno directory.
> If people cared about them still they would have submitted for
> specs/kilo.
>
> So any objection to killing everything in the list below:
>
>
> +-+---+--+---+-+--+
> | URL | Subject
>| Created  | Tests | Reviews | Workflow |
>
> +-+---+--+---+-+--+
> | https://review.openstack.org/86938  | Add tasks to the v3 API
>  | 237 days |  1| -2  |  |
> | https://review.openstack.org/88334  | Add support for USB controller
> | 231 days |  1| -2  |  |
> | https://review.openstack.org/89766  | Add useful metrics into
> utilization based scheduli... | 226 days |  1| -2  |  |
> | https://review.openstack.org/90239  | Blueprint for Cinder Multi attach
> volumes | 224 days |  1| -2  |  |
> | https://review.openstack.org/90647  | Add utilization based weighers on
> top of MetricsWe... | 221 days |  1| -2  |  |
> | https://review.openstack.org/96543  | Smart Scheduler (Solver
> Scheduler) - Constraint ba... | 189 days |  1| -2  |  |
> | https://review.openstack.org/97441  | Add nova spec for
> bp/isnot-operator   | 185 days |  1| -2  |
> |
> | https://review.openstack.org/99476  | Dedicate aggregates for specific
> tenants  | 176 days |  1| -2  |  |
> | https://review.openstack.org/99576  | Add client token to CreateServer
> | 176 days |  1| -2  |  |
> | https://review.openstack.org/101921 | Spec for Neutron migration
> feature| 164 days |  1| -2  |  |
> | https://review.openstack.org/103617 | Support Identity V3 API
>  | 157 days |  1| -1  |  |
> | https://review.openstack.org/105385 | Leverage the features of IBM GPFS
> to store cached ... | 150 days |  1| -2  |  |
> | https://review.openstack.org/108582 | Add ironic boot mode filters
> | 136 days |  1| -2  |  |
> | https://review.openstack.org/110639 | Blueprint for the implementation
> of Nested Quota D... | 127 days |  1| -2  |  |
> | https://review.openstack.org/111308 | Added VirtProperties object
> blueprint | 125 days |  1| -2  |  |
> | https://review.openstack.org/111745 | Improve instance boot time
> | 122 days |  1| -2  |  |
> | https://review.openstack.org/116280 | Add a new filter to implement
> project isolation fe... | 104 days |  1| -2  |  |
>
> +-+---+--+---+-+--+
>
>
> Regards,
> Daniel
> --
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/
> :|
> |: http://libvirt.org  -o- http://virt-manager.org
> :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/
> :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc
> :|
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Daniel P. Berrange]

On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> Nova currently has 197 patches that have seen no activity in the last 4
> weeks (project:openstack/nova age:4weeks status:open).

On a somewhat related note, nova-specs currently has 17 specs
open against specs/juno, most with -2 votes. I think we should
just mass-abandon anything still touching the specs/juno directory.
If people cared about them still they would have submitted for
specs/kilo.

So any objection to killing everything in the list below:

+-+---+--+---+-+--+
| URL | Subject 
  | Created  | Tests | Reviews | Workflow |
+-+---+--+---+-+--+
| https://review.openstack.org/86938  | Add tasks to the v3 API 
  | 237 days |  1| -2  |  |
| https://review.openstack.org/88334  | Add support for USB controller  
  | 231 days |  1| -2  |  |
| https://review.openstack.org/89766  | Add useful metrics into utilization 
based scheduli... | 226 days |  1| -2  |  |
| https://review.openstack.org/90239  | Blueprint for Cinder Multi attach 
volumes | 224 days |  1| -2  |  |
| https://review.openstack.org/90647  | Add utilization based weighers on top 
of MetricsWe... | 221 days |  1| -2  |  |
| https://review.openstack.org/96543  | Smart Scheduler (Solver Scheduler) - 
Constraint ba... | 189 days |  1| -2  |  |
| https://review.openstack.org/97441  | Add nova spec for bp/isnot-operator 
  | 185 days |  1| -2  |  |
| https://review.openstack.org/99476  | Dedicate aggregates for specific 
tenants  | 176 days |  1| -2  |  |
| https://review.openstack.org/99576  | Add client token to CreateServer
  | 176 days |  1| -2  |  |
| https://review.openstack.org/101921 | Spec for Neutron migration feature  
  | 164 days |  1| -2  |  |
| https://review.openstack.org/103617 | Support Identity V3 API 
  | 157 days |  1| -1  |  |
| https://review.openstack.org/105385 | Leverage the features of IBM GPFS to 
store cached ... | 150 days |  1| -2  |  |
| https://review.openstack.org/108582 | Add ironic boot mode filters
  | 136 days |  1| -2  |  |
| https://review.openstack.org/110639 | Blueprint for the implementation of 
Nested Quota D... | 127 days |  1| -2  |  |
| https://review.openstack.org/111308 | Added VirtProperties object blueprint   
  | 125 days |  1| -2  |  |
| https://review.openstack.org/111745 | Improve instance boot time  
  | 122 days |  1| -2  |  |
| https://review.openstack.org/116280 | Add a new filter to implement project 
isolation fe... | 104 days |  1| -2  |  |
+-+---+--+---+-+--+


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Tony Breeds]

On Tue, Nov 18, 2014 at 07:33:54AM -0500, Sean Dague wrote:
> Or just abandon and let people restore. I think handling the logic /
> policy for the edge cases isn't worth it when the author can very easily
> hit the restore button to get their patch back (and fresh for another 4w).

As a non-core developer it seems like a small price to pay to help active
patches stay visible.

Yours Tony.


pgp1NgJEXHNdL.pgp
Description: PGP signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Joe Gordon]

On Tue, Nov 18, 2014 at 6:17 AM, Daniel P. Berrange 
wrote:

> On Tue, Nov 18, 2014 at 07:33:54AM -0500, Sean Dague wrote:
> > On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:
> > > On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> > >> Nova currently has 197 patches that have seen no activity in the last
> 4
> > >> weeks (project:openstack/nova age:4weeks status:open).
> > >>
> > >> Of these
> > >>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
> > >> status:open label:Verified<=-1,jenkins)
> > >>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
> > >> status:open label:Code-Review<=-2)
> > >>
> > >> (note, those 2 groups sometimes overlap)
> > >>
> > >> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
> > >> of them seem dead, seems like a cleanup thing we could do.
> > >>
> > >> I'd like to propose that we implement our own auto abandon mechanism
> > >> based on reviews that are either held by a -2, or Jenkins -1 after 4
> > >> weeks time. I can write a quick script to abandon with a friendly
> > >> message about why we are doing it, and to restore it if work is
> continuing.
> > > Yep, purging anything that's older than 4 weeks with negative karma
> > > seems like a good idea.  It'll make it easier for us to identify those
> > > patches which are still "maintained" and target them for review.
> > >
> > > That said, there's some edge cases - for example I've got some patches
> > > up for review that have a -2 on them, becase we're waiting for
> blueprint
> > > approval. IIRC, previously we would post a warning about pending auto-
> > > abandon a week before, and thus give the author the chance to add a
> > > comment to prevent auto-abandon taking place. It would be neccessary to
> > > have this ability to deal with the case where we're just temporarily
> > > blocked on other work.
> > >
> > > Also sometimes when you have a large patch series, you might have some
> > > patches later in the series which (temporarily) fail the jenkins jobs.
> > > It often isn't worth fixing those failures until you have dealt with
> > > review earlier in the patch series. So I think we should not
> auto-expire
> > > patches which are in the middle of a patch series, unless the
> preceeding
> > > patches in the series are to be expired too.  Yes this isn't something
> > > you can figure out with a single gerrit query - you'd have to query
> > > gerrit for patches and then look at the parent change references.
> > Or just abandon and let people restore. I think handling the logic /
> > policy for the edge cases isn't worth it when the author can very easily
> > hit the restore button to get their patch back (and fresh for another
> 4w).
> >
> > If it was a large patch series, this wouldn't happen anyway, because
> > every rebase would make it fresh. 4w is really 4w of nothing changing.
>
> Ok, that makes sense and is workable I reckon.
>

++ for bringing back auto abandon in this model.


>
> Regards,
> Daniel
> --
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/
> :|
> |: http://libvirt.org  -o- http://virt-manager.org
> :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/
> :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc
> :|
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Christopher Yeoh]

On Tue, Nov 18, 2014 at 11:44 PM, Jay Pipes  wrote:

> On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:
>
>> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
>>
>>> Nova currently has 197 patches that have seen no activity in the last 4
>>> weeks (project:openstack/nova age:4weeks status:open).
>>>
>>> Of these
>>>   * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
>>> status:open label:Verified<=-1,jenkins)
>>>   * 60 are -2 by a core team member (project:openstack/nova age:4weeks
>>> status:open label:Code-Review<=-2)
>>>
>>> (note, those 2 groups sometimes overlap)
>>>
>>> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
>>> of them seem dead, seems like a cleanup thing we could do.
>>>
>>> I'd like to propose that we implement our own auto abandon mechanism
>>> based on reviews that are either held by a -2, or Jenkins -1 after 4
>>> weeks time. I can write a quick script to abandon with a friendly
>>> message about why we are doing it, and to restore it if work is
>>> continuing.
>>>
>>
>> Yep, purging anything that's older than 4 weeks with negative karma
>> seems like a good idea.  It'll make it easier for us to identify those
>> patches which are still "maintained" and target them for review.
>>
>> That said, there's some edge cases - for example I've got some patches
>> up for review that have a -2 on them, becase we're waiting for blueprint
>> approval. IIRC, previously we would post a warning about pending auto-
>> abandon a week before, and thus give the author the chance to add a
>> comment to prevent auto-abandon taking place. It would be neccessary to
>> have this ability to deal with the case where we're just temporarily
>> blocked on other work.
>>
>
> Yes, this is indeed an issue. However, couldn't we just say "Add a
> -Workflow label to avoid auto-abandon" and then have the script simply
> ignore patches with -Workflow?


+1 for not auto abandoning patches marked WIP (or at least make the timeout
much longer for those)

Chris
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Daniel P. Berrange]

On Tue, Nov 18, 2014 at 07:33:54AM -0500, Sean Dague wrote:
> On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:
> > On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> >> Nova currently has 197 patches that have seen no activity in the last 4
> >> weeks (project:openstack/nova age:4weeks status:open).
> >>
> >> Of these
> >>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
> >> status:open label:Verified<=-1,jenkins)
> >>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
> >> status:open label:Code-Review<=-2)
> >>
> >> (note, those 2 groups sometimes overlap)
> >>
> >> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
> >> of them seem dead, seems like a cleanup thing we could do.
> >>
> >> I'd like to propose that we implement our own auto abandon mechanism
> >> based on reviews that are either held by a -2, or Jenkins -1 after 4
> >> weeks time. I can write a quick script to abandon with a friendly
> >> message about why we are doing it, and to restore it if work is continuing.
> > Yep, purging anything that's older than 4 weeks with negative karma
> > seems like a good idea.  It'll make it easier for us to identify those
> > patches which are still "maintained" and target them for review.
> >
> > That said, there's some edge cases - for example I've got some patches
> > up for review that have a -2 on them, becase we're waiting for blueprint
> > approval. IIRC, previously we would post a warning about pending auto-
> > abandon a week before, and thus give the author the chance to add a
> > comment to prevent auto-abandon taking place. It would be neccessary to
> > have this ability to deal with the case where we're just temporarily
> > blocked on other work.
> >
> > Also sometimes when you have a large patch series, you might have some
> > patches later in the series which (temporarily) fail the jenkins jobs.
> > It often isn't worth fixing those failures until you have dealt with
> > review earlier in the patch series. So I think we should not auto-expire
> > patches which are in the middle of a patch series, unless the preceeding
> > patches in the series are to be expired too.  Yes this isn't something
> > you can figure out with a single gerrit query - you'd have to query
> > gerrit for patches and then look at the parent change references.
> Or just abandon and let people restore. I think handling the logic /
> policy for the edge cases isn't worth it when the author can very easily
> hit the restore button to get their patch back (and fresh for another 4w).
> 
> If it was a large patch series, this wouldn't happen anyway, because
> every rebase would make it fresh. 4w is really 4w of nothing changing.

Ok, that makes sense and is workable I reckon.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Jay Pipes]

On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:

On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:

Nova currently has 197 patches that have seen no activity in the last 4
weeks (project:openstack/nova age:4weeks status:open).

Of these
  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
status:open label:Verified<=-1,jenkins)
  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
status:open label:Code-Review<=-2)

(note, those 2 groups sometimes overlap)

Regardless, the fact that Nova currently has 792 open reviews, and 1/4
of them seem dead, seems like a cleanup thing we could do.

I'd like to propose that we implement our own auto abandon mechanism
based on reviews that are either held by a -2, or Jenkins -1 after 4
weeks time. I can write a quick script to abandon with a friendly
message about why we are doing it, and to restore it if work is continuing.


Yep, purging anything that's older than 4 weeks with negative karma
seems like a good idea.  It'll make it easier for us to identify those
patches which are still "maintained" and target them for review.

That said, there's some edge cases - for example I've got some patches
up for review that have a -2 on them, becase we're waiting for blueprint
approval. IIRC, previously we would post a warning about pending auto-
abandon a week before, and thus give the author the chance to add a
comment to prevent auto-abandon taking place. It would be neccessary to
have this ability to deal with the case where we're just temporarily
blocked on other work.


Yes, this is indeed an issue. However, couldn't we just say "Add a 
-Workflow label to avoid auto-abandon" and then have the script simply 
ignore patches with -Workflow? As you note below, it's not a simple 
thing to determine all of the patches in a series that may happen to 
have a -Workflow at the "tail" of the patch series, but it's probably 
doable with a little script-fu.


Best,
-jay


Also sometimes when you have a large patch series, you might have some
patches later in the series which (temporarily) fail the jenkins jobs.
It often isn't worth fixing those failures until you have dealt with
review earlier in the patch series. So I think we should not auto-expire
patches which are in the middle of a patch series, unless the preceeding
patches in the series are to be expired too.  Yes this isn't something
you can figure out with a single gerrit query - you'd have to query
gerrit for patches and then look at the parent change references.



Regards,
Daniel



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Sean Dague]

On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:
> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
>> Nova currently has 197 patches that have seen no activity in the last 4
>> weeks (project:openstack/nova age:4weeks status:open).
>>
>> Of these
>>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
>> status:open label:Verified<=-1,jenkins)
>>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
>> status:open label:Code-Review<=-2)
>>
>> (note, those 2 groups sometimes overlap)
>>
>> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
>> of them seem dead, seems like a cleanup thing we could do.
>>
>> I'd like to propose that we implement our own auto abandon mechanism
>> based on reviews that are either held by a -2, or Jenkins -1 after 4
>> weeks time. I can write a quick script to abandon with a friendly
>> message about why we are doing it, and to restore it if work is continuing.
> Yep, purging anything that's older than 4 weeks with negative karma
> seems like a good idea.  It'll make it easier for us to identify those
> patches which are still "maintained" and target them for review.
>
> That said, there's some edge cases - for example I've got some patches
> up for review that have a -2 on them, becase we're waiting for blueprint
> approval. IIRC, previously we would post a warning about pending auto-
> abandon a week before, and thus give the author the chance to add a
> comment to prevent auto-abandon taking place. It would be neccessary to
> have this ability to deal with the case where we're just temporarily
> blocked on other work.
>
> Also sometimes when you have a large patch series, you might have some
> patches later in the series which (temporarily) fail the jenkins jobs.
> It often isn't worth fixing those failures until you have dealt with
> review earlier in the patch series. So I think we should not auto-expire
> patches which are in the middle of a patch series, unless the preceeding
> patches in the series are to be expired too.  Yes this isn't something
> you can figure out with a single gerrit query - you'd have to query
> gerrit for patches and then look at the parent change references.
Or just abandon and let people restore. I think handling the logic /
policy for the edge cases isn't worth it when the author can very easily
hit the restore button to get their patch back (and fresh for another 4w).

If it was a large patch series, this wouldn't happen anyway, because
every rebase would make it fresh. 4w is really 4w of nothing changing.

-Sean

-- 
Sean Dague
http://dague.net




signature.asc
Description: OpenPGP digital signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Daniel P. Berrange]

On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
> Nova currently has 197 patches that have seen no activity in the last 4
> weeks (project:openstack/nova age:4weeks status:open).
> 
> Of these
>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
> status:open label:Verified<=-1,jenkins)
>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
> status:open label:Code-Review<=-2)
> 
> (note, those 2 groups sometimes overlap)
> 
> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
> of them seem dead, seems like a cleanup thing we could do.
> 
> I'd like to propose that we implement our own auto abandon mechanism
> based on reviews that are either held by a -2, or Jenkins -1 after 4
> weeks time. I can write a quick script to abandon with a friendly
> message about why we are doing it, and to restore it if work is continuing.

Yep, purging anything that's older than 4 weeks with negative karma
seems like a good idea.  It'll make it easier for us to identify those
patches which are still "maintained" and target them for review.

That said, there's some edge cases - for example I've got some patches
up for review that have a -2 on them, becase we're waiting for blueprint
approval. IIRC, previously we would post a warning about pending auto-
abandon a week before, and thus give the author the chance to add a
comment to prevent auto-abandon taking place. It would be neccessary to
have this ability to deal with the case where we're just temporarily
blocked on other work.

Also sometimes when you have a large patch series, you might have some
patches later in the series which (temporarily) fail the jenkins jobs.
It often isn't worth fixing those failures until you have dealt with
review earlier in the patch series. So I think we should not auto-expire
patches which are in the middle of a patch series, unless the preceeding
patches in the series are to be expired too.  Yes this isn't something
you can figure out with a single gerrit query - you'd have to query
gerrit for patches and then look at the parent change references.



Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] policy on old / virtually abandoned patches

[Ken'ichi Ohmichi]

2014-11-18 21:06 GMT+09:00 Sean Dague :
> Nova currently has 197 patches that have seen no activity in the last 4
> weeks (project:openstack/nova age:4weeks status:open).
>
> Of these
>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
> status:open label:Verified<=-1,jenkins)
>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
> status:open label:Code-Review<=-2)
>
> (note, those 2 groups sometimes overlap)
>
> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
> of them seem dead, seems like a cleanup thing we could do.
>
> I'd like to propose that we implement our own auto abandon mechanism
> based on reviews that are either held by a -2, or Jenkins -1 after 4
> weeks time. I can write a quick script to abandon with a friendly
> message about why we are doing it, and to restore it if work is continuing.

thanks Sean, +1 for doing this.

Thanks
Ken Ohmichi

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev