Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-09 Thread Colleen Murphy
On Tue, Jun 2, 2015 at 11:39 AM, Colleen Murphy  wrote:

> In today's meeting we discussed implementing a policy for whether and when
> core reviewers should abandon old patches whose author's were inactive.
> (This doesn't apply to authors that want to abandon their own changes, only
> for core reviewers to abandon other people's changes.) There are a few
> things we could do here, with potential policy drafts for the wiki:
>
> 1) Never abandon
>
> ```
> Our policy is to never abandon changes except for our own.
> ```
>
> The sentiment here is that an old change in the queue isn't really hurting
> anything by just sitting there, and it is more visible if someone else
> wants to pick up the change.
>
> 2) Manually abandon after N months/weeks changes that have a -2 or were
> fixed in a different patch
>
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. Core reviewers will not abandon such a change.
>
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
> ```
>
> Core reviewers can click the abandon button only on old patches that are
> definitely never going to make it in. This approach has the advantage that
> it is easier for contributors to find changes and fix them up, even if the
> change is very old.
>
> 3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
>
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. If the author is unresponsive for at least 3 months
> and no one else takes over the patch, core reviewers can abandon the patch,
> leaving a detailed note about how the change can be restored.
>
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
> ```
>
> Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs does
> this for its module pull requests, setting N at 1 month.
>
> 4) Auto-abandon after N months/weeks if patch has a -1 or -2
>
> ```
> If a change is given a -2 and the author has been unresponsive for at
> least 3 months, a script will automatically abandon the change, leaving a
> message about how the author can restore the change and attempt to resolve
> the -2 with the reviewer who left it.
> ```
>
> We would use a tool like this one[1] to automatically abandon changes
> meeting a certain criteria. We would have to decide whether we want to only
> auto-abandon changes with -2's or go as far as to auto-abandon those with
> -1's. The policy proposal above assumes -2. The tool would leave a canned
> message about how to restore the change.
>
>
> Option 1 has the problem of leaving clutter around, which the discussion
> today seeks to solve.
>
> Option 3 leaves the possibility that a change that is mostly good becomes
> abandoned, making it harder for someone to find and restore it.
>
>  I don't think option 4 is necessary because there are not an overwhelming
> number of old changes (I count 9 that are currently over six months old).
> In working through old changes a few months ago I found that many of them
> are easy to fix up to remove a -1, and auto-abandoning removes the ability
> for a human to make that call. Moreover, if a patch has a procedural -2
> that ought to be lifted after some point, auto-abandonment has the
> potential to accidentally throw out a change that was intended to be kept
> (though presumably the core reviewer who left the -2 would notice the
> abandonment and restore it if that was the case).
>
> I am in favor of option 2. I think setting N to be 3 months or 6 months is
> appropriate. I don't have very strong feelings about options 1 or 3. I'm
> against option 4.
>
> Colleen
>
> [1]
> https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh
>
Though we didn't seem to reach a clear consensus in this thre

Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-06 Thread Emilien Macchi


On 06/02/2015 02:39 PM, Colleen Murphy wrote:
> In today's meeting we discussed implementing a policy for whether and
> when core reviewers should abandon old patches whose author's were
> inactive. (This doesn't apply to authors that want to abandon their own
> changes, only for core reviewers to abandon other people's changes.)
> There are a few things we could do here, with potential policy drafts
> for the wiki:
> 
> 1) Never abandon
> 
> ```
> Our policy is to never abandon changes except for our own.
> ```
> 
> The sentiment here is that an old change in the queue isn't really
> hurting anything by just sitting there, and it is more visible if
> someone else wants to pick up the change.
> 
> 2) Manually abandon after N months/weeks changes that have a -2 or were
> fixed in a different patch
> 
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC
> or email. If the change is easy to fix, anyone should feel welcome to
> check out the change and resubmit it using the same change ID to
> preserve original authorship. Core reviewers will not abandon such a change.
> 
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change
> was chosen to solve the problem), and the author has been unresponsive
> for at least 3 months, a core reviewer should abandon the change.
> ```
> 
> Core reviewers can click the abandon button only on old patches that are
> definitely never going to make it in. This approach has the advantage
> that it is easier for contributors to find changes and fix them up, even
> if the change is very old.
> 
> 3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
> 
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC
> or email. If the change is easy to fix, anyone should feel welcome to
> check out the change and resubmit it using the same change ID to
> preserve original authorship. If the author is unresponsive for at least
> 3 months and no one else takes over the patch, core reviewers can
> abandon the patch, leaving a detailed note about how the change can be
> restored.
> 
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change
> was chosen to solve the problem), and the author has been unresponsive
> for at least 3 months, a core reviewer should abandon the change.
> ```
> 
> Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs
> does this for its module pull requests, setting N at 1 month.
> 
> 4) Auto-abandon after N months/weeks if patch has a -1 or -2
> 
> ```
> If a change is given a -2 and the author has been unresponsive for at
> least 3 months, a script will automatically abandon the change, leaving
> a message about how the author can restore the change and attempt to
> resolve the -2 with the reviewer who left it.
> ```
> 
> We would use a tool like this one[1] to automatically abandon changes
> meeting a certain criteria. We would have to decide whether we want to
> only auto-abandon changes with -2's or go as far as to auto-abandon
> those with -1's. The policy proposal above assumes -2. The tool would
> leave a canned message about how to restore the change.
> 
> 
> Option 1 has the problem of leaving clutter around, which the discussion
> today seeks to solve.
> 
> Option 3 leaves the possibility that a change that is mostly good
> becomes abandoned, making it harder for someone to find and restore it.
> 
>  I don't think option 4 is necessary because there are not an
> overwhelming number of old changes (I count 9 that are currently over
> six months old). In working through old changes a few months ago I found
> that many of them are easy to fix up to remove a -1, and auto-abandoning
> removes the ability for a human to make that call. Moreover, if a patch
> has a procedural -2 that ought to be lifted after some point,
> auto-abandonment has the potential to accidentally throw out a change
> that was intended to be kept (though presumably the core reviewer who
> left the -2 would notice the abandonment and restore it if that was the
> case). 
> 
> I am in favor of option 2. I think setting N to be 3 months or 6 months
> is appropriate. I don't have very strong feelings about options 1 or 3.
> I'm against option 4.

#2 is good indeed, it also avoid to discourage new contributors that
need more time to provide a good patch.

Like you said, some OpenStack pr

Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-05 Thread Boris Pavlovic
Hi,

+1 for #1 and if patch is not touched for N weeks just finish it using
current active team.

Best regards,
Boris Pavlovic

On Fri, Jun 5, 2015 at 7:27 PM, Richard Raseley  wrote:

> Colleen Murphy wrote:
>
>> 3) Manually abandon after N months/weeks changes that have a -1 that was
>> never responded to
>>
>> ```
>> If a change is submitted and given a -1, and subsequently the author
>> becomes unresponsive for a few weeks, reviewers should leave reminder
>> comments on the review or attempt to contact the original author via IRC
>> or email. If the change is easy to fix, anyone should feel welcome to
>> check out the change and resubmit it using the same change ID to
>> preserve original authorship. If the author is unresponsive for at least
>> 3 months and no one else takes over the patch, core reviewers can
>> abandon the patch, leaving a detailed note about how the change can be
>> restored.
>>
>> If a change is submitted and given a -2, or it otherwise becomes clear
>> that the change can not make it in (for example, if an alternate change
>> was chosen to solve the problem), and the author has been unresponsive
>> for at least 3 months, a core reviewer should abandon the change.
>> ```
>>
>
> +1 for #3
>
> __
> 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] [puppet] Change abandonment policy

2015-06-05 Thread Richard Raseley

Colleen Murphy wrote:

3) Manually abandon after N months/weeks changes that have a -1 that was
never responded to

```
If a change is submitted and given a -1, and subsequently the author
becomes unresponsive for a few weeks, reviewers should leave reminder
comments on the review or attempt to contact the original author via IRC
or email. If the change is easy to fix, anyone should feel welcome to
check out the change and resubmit it using the same change ID to
preserve original authorship. If the author is unresponsive for at least
3 months and no one else takes over the patch, core reviewers can
abandon the patch, leaving a detailed note about how the change can be
restored.

If a change is submitted and given a -2, or it otherwise becomes clear
that the change can not make it in (for example, if an alternate change
was chosen to solve the problem), and the author has been unresponsive
for at least 3 months, a core reviewer should abandon the change.
```


+1 for #3

__
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] [puppet] Change abandonment policy

2015-06-05 Thread Sanjay Upadhyay
+1 for #3 with N = 1

regards
/sanjay

On Fri, Jun 5, 2015 at 1:27 AM, Mike Dorman  wrote:

>   I vote #2, with a smaller N.
>
>  We can always adjust this policy in the future if find we have to
> manually abandon too many old reviews.
>
>
>   From: Colleen Murphy
> Reply-To: "puppet-openst...@puppetlabs.com"
> Date: Tuesday, June 2, 2015 at 12:39 PM
> To: "OpenStack Development Mailing List (not for usage questions)", "
> puppet-openst...@puppetlabs.com"
> Subject: [puppet] Change abandonment policy
>
>   In today's meeting we discussed implementing a policy for whether and
> when core reviewers should abandon old patches whose author's were
> inactive. (This doesn't apply to authors that want to abandon their own
> changes, only for core reviewers to abandon other people's changes.) There
> are a few things we could do here, with potential policy drafts for the
> wiki:
>
>  1) Never abandon
>
>  ```
> Our policy is to never abandon changes except for our own.
> ```
>
>  The sentiment here is that an old change in the queue isn't really
> hurting anything by just sitting there, and it is more visible if someone
> else wants to pick up the change.
>
>  2) Manually abandon after N months/weeks changes that have a -2 or were
> fixed in a different patch
>
>  ```
>  If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. Core reviewers will not abandon such a change.
>
>  If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
>  ```
>
>  Core reviewers can click the abandon button only on old patches that are
> definitely never going to make it in. This approach has the advantage that
> it is easier for contributors to find changes and fix them up, even if the
> change is very old.
>
>  3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
>
>  ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. If the author is unresponsive for at least 3 months
> and no one else takes over the patch, core reviewers can abandon the patch,
> leaving a detailed note about how the change can be restored.
>
>  If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
> ```
>
>  Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs does
> this for its module pull requests, setting N at 1 month.
>
>  4) Auto-abandon after N months/weeks if patch has a -1 or -2
>
>  ```
> If a change is given a -2 and the author has been unresponsive for at
> least 3 months, a script will automatically abandon the change, leaving a
> message about how the author can restore the change and attempt to resolve
> the -2 with the reviewer who left it.
> ```
>
>  We would use a tool like this one[1] to automatically abandon changes
> meeting a certain criteria. We would have to decide whether we want to only
> auto-abandon changes with -2's or go as far as to auto-abandon those with
> -1's. The policy proposal above assumes -2. The tool would leave a canned
> message about how to restore the change.
>
>
>  Option 1 has the problem of leaving clutter around, which the discussion
> today seeks to solve.
>
>  Option 3 leaves the possibility that a change that is mostly good
> becomes abandoned, making it harder for someone to find and restore it.
>
>   I don't think option 4 is necessary because there are not an
> overwhelming number of old changes (I count 9 that are currently over six
> months old). In working through old changes a few months ago I found that
> many of them are easy to fix up to remove a -1, and auto-abandoning removes
> the ability for a human to make that call. Moreover, if a patch has a
> procedural -2 that ought to be lifted after some point, auto-abandonment
> has the potential to accidentally throw out a change that was int

Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-04 Thread Mike Dorman
I vote #2, with a smaller N.

We can always adjust this policy in the future if find we have to manually 
abandon too many old reviews.


From: Colleen Murphy
Reply-To: 
"puppet-openst...@puppetlabs.com"
Date: Tuesday, June 2, 2015 at 12:39 PM
To: "OpenStack Development Mailing List (not for usage questions)", 
"puppet-openst...@puppetlabs.com"
Subject: [puppet] Change abandonment policy

In today's meeting we discussed implementing a policy for whether and when core 
reviewers should abandon old patches whose author's were inactive. (This 
doesn't apply to authors that want to abandon their own changes, only for core 
reviewers to abandon other people's changes.) There are a few things we could 
do here, with potential policy drafts for the wiki:

1) Never abandon

```
Our policy is to never abandon changes except for our own.
```

The sentiment here is that an old change in the queue isn't really hurting 
anything by just sitting there, and it is more visible if someone else wants to 
pick up the change.

2) Manually abandon after N months/weeks changes that have a -2 or were fixed 
in a different patch

```
If a change is submitted and given a -1, and subsequently the author becomes 
unresponsive for a few weeks, reviewers should leave reminder comments on the 
review or attempt to contact the original author via IRC or email. If the 
change is easy to fix, anyone should feel welcome to check out the change and 
resubmit it using the same change ID to preserve original authorship. Core 
reviewers will not abandon such a change.

If a change is submitted and given a -2, or it otherwise becomes clear that the 
change can not make it in (for example, if an alternate change was chosen to 
solve the problem), and the author has been unresponsive for at least 3 months, 
a core reviewer should abandon the change.
```

Core reviewers can click the abandon button only on old patches that are 
definitely never going to make it in. This approach has the advantage that it 
is easier for contributors to find changes and fix them up, even if the change 
is very old.

3) Manually abandon after N months/weeks changes that have a -1 that was never 
responded to

```
If a change is submitted and given a -1, and subsequently the author becomes 
unresponsive for a few weeks, reviewers should leave reminder comments on the 
review or attempt to contact the original author via IRC or email. If the 
change is easy to fix, anyone should feel welcome to check out the change and 
resubmit it using the same change ID to preserve original authorship. If the 
author is unresponsive for at least 3 months and no one else takes over the 
patch, core reviewers can abandon the patch, leaving a detailed note about how 
the change can be restored.

If a change is submitted and given a -2, or it otherwise becomes clear that the 
change can not make it in (for example, if an alternate change was chosen to 
solve the problem), and the author has been unresponsive for at least 3 months, 
a core reviewer should abandon the change.
```

Core reviewers can click the abandon button on changes that no one has shown an 
interest in in N months/weeks, leaving a message about how to restore the 
change if the author wants to come back to it. Puppet Labs does this for its 
module pull requests, setting N at 1 month.

4) Auto-abandon after N months/weeks if patch has a -1 or -2

```
If a change is given a -2 and the author has been unresponsive for at least 3 
months, a script will automatically abandon the change, leaving a message about 
how the author can restore the change and attempt to resolve the -2 with the 
reviewer who left it.
```

We would use a tool like this one[1] to automatically abandon changes meeting a 
certain criteria. We would have to decide whether we want to only auto-abandon 
changes with -2's or go as far as to auto-abandon those with -1's. The policy 
proposal above assumes -2. The tool would leave a canned message about how to 
restore the change.


Option 1 has the problem of leaving clutter around, which the discussion today 
seeks to solve.

Option 3 leaves the possibility that a change that is mostly good becomes 
abandoned, making it harder for someone to find and restore it.

 I don't think option 4 is necessary because there are not an overwhelming 
number of old changes (I count 9 that are currently over six months old). In 
working through old changes a few months ago I found that many of them are easy 
to fix up to remove a -1, and auto-abandoning removes the ability for a human 
to make that call. Moreover, if a patch has a procedural -2 that ought to be 
lifted after some point, auto-abandonment has the potential to accidentally 
throw out a change that was intended to be kept (though presumably the core 
reviewer who left the -2 would notice the abandonment and restore it if that 
was the case).

I am in favor of option 2. I think se

Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-03 Thread Martin Mágr


On 06/02/2015 08:39 PM, Colleen Murphy wrote:

4) Auto-abandon after N months/weeks if patch has a -1 or -2

```
If a change is given a -2 and the author has been unresponsive for at 
least 3 months, a script will automatically abandon the change, 
leaving a message about how the author can restore the change and 
attempt to resolve the -2 with the reviewer who left it.

```

We would use a tool like this one[1] to automatically abandon changes 
meeting a certain criteria. We would have to decide whether we want to 
only auto-abandon changes with -2's or go as far as to auto-abandon 
those with -1's. The policy proposal above assumes -2. The tool would 
leave a canned message about how to restore the change.
+1 for auto-abandoning. 3-4 weeks inactivity with -1/-2 seems reasonable 
proof that commiter gave up on patch.


Martin

__
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] [puppet] Change abandonment policy

2015-06-02 Thread Cody Herriges
Colleen Murphy wrote:

> 
> 3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
> 
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC
> or email. If the change is easy to fix, anyone should feel welcome to
> check out the change and resubmit it using the same change ID to
> preserve original authorship. If the author is unresponsive for at least
> 3 months and no one else takes over the patch, core reviewers can
> abandon the patch, leaving a detailed note about how the change can be
> restored.
> 
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change
> was chosen to solve the problem), and the author has been unresponsive
> for at least 3 months, a core reviewer should abandon the change.
> ```
> 
> Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs
> does this for its module pull requests, setting N at 1 month.
> 

+1

> 
> Option 3 leaves the possibility that a change that is mostly good
> becomes abandoned, making it harder for someone to find and restore it.
> 

In my opinion this will happen very infrequently.

-- 
Cody



signature.asc
Description: OpenPGP digital signature
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [puppet] Change abandonment policy

2015-06-02 Thread Andrew Woodward
Also in favor of #2 and thought it was how it was running. #4 sounds bad
and may hide good code.

How do we want to account for drive-by authors who are going to be unable
to work on future revisions. We talked a while back that we wanted to be
able to account for this as some operators are unable to do/have time for
proper CR cycles to get patches landed.

On Tue, Jun 2, 2015 at 11:39 AM Colleen Murphy  wrote:

> In today's meeting we discussed implementing a policy for whether and when
> core reviewers should abandon old patches whose author's were inactive.
> (This doesn't apply to authors that want to abandon their own changes, only
> for core reviewers to abandon other people's changes.) There are a few
> things we could do here, with potential policy drafts for the wiki:
>
> 1) Never abandon
>
> ```
> Our policy is to never abandon changes except for our own.
> ```
>
> The sentiment here is that an old change in the queue isn't really hurting
> anything by just sitting there, and it is more visible if someone else
> wants to pick up the change.
>
> 2) Manually abandon after N months/weeks changes that have a -2 or were
> fixed in a different patch
>
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. Core reviewers will not abandon such a change.
>
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
> ```
>
> Core reviewers can click the abandon button only on old patches that are
> definitely never going to make it in. This approach has the advantage that
> it is easier for contributors to find changes and fix them up, even if the
> change is very old.
>
> 3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
>
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC or
> email. If the change is easy to fix, anyone should feel welcome to check
> out the change and resubmit it using the same change ID to preserve
> original authorship. If the author is unresponsive for at least 3 months
> and no one else takes over the patch, core reviewers can abandon the patch,
> leaving a detailed note about how the change can be restored.
>
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change was
> chosen to solve the problem), and the author has been unresponsive for at
> least 3 months, a core reviewer should abandon the change.
> ```
>
> Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs does
> this for its module pull requests, setting N at 1 month.
>
> 4) Auto-abandon after N months/weeks if patch has a -1 or -2
>
> ```
> If a change is given a -2 and the author has been unresponsive for at
> least 3 months, a script will automatically abandon the change, leaving a
> message about how the author can restore the change and attempt to resolve
> the -2 with the reviewer who left it.
> ```
>
> We would use a tool like this one[1] to automatically abandon changes
> meeting a certain criteria. We would have to decide whether we want to only
> auto-abandon changes with -2's or go as far as to auto-abandon those with
> -1's. The policy proposal above assumes -2. The tool would leave a canned
> message about how to restore the change.
>
>
> Option 1 has the problem of leaving clutter around, which the discussion
> today seeks to solve.
>
> Option 3 leaves the possibility that a change that is mostly good becomes
> abandoned, making it harder for someone to find and restore it.
>
>  I don't think option 4 is necessary because there are not an overwhelming
> number of old changes (I count 9 that are currently over six months old).
> In working through old changes a few months ago I found that many of them
> are easy to fix up to remove a -1, and auto-abandoning removes the ability
> for a human to make that call. Moreover, if a patch has a procedural -2
> that ought to be lifted after some point, auto-abandonment has the
> potential to accidentally throw out a change that was intended to be kept
> (though presumably the core reviewer who left the -2 would notice the
> abandonment and restore i

[openstack-dev] [puppet] Change abandonment policy

2015-06-02 Thread Colleen Murphy
In today's meeting we discussed implementing a policy for whether and when
core reviewers should abandon old patches whose author's were inactive.
(This doesn't apply to authors that want to abandon their own changes, only
for core reviewers to abandon other people's changes.) There are a few
things we could do here, with potential policy drafts for the wiki:

1) Never abandon

```
Our policy is to never abandon changes except for our own.
```

The sentiment here is that an old change in the queue isn't really hurting
anything by just sitting there, and it is more visible if someone else
wants to pick up the change.

2) Manually abandon after N months/weeks changes that have a -2 or were
fixed in a different patch

```
If a change is submitted and given a -1, and subsequently the author
becomes unresponsive for a few weeks, reviewers should leave reminder
comments on the review or attempt to contact the original author via IRC or
email. If the change is easy to fix, anyone should feel welcome to check
out the change and resubmit it using the same change ID to preserve
original authorship. Core reviewers will not abandon such a change.

If a change is submitted and given a -2, or it otherwise becomes clear that
the change can not make it in (for example, if an alternate change was
chosen to solve the problem), and the author has been unresponsive for at
least 3 months, a core reviewer should abandon the change.
```

Core reviewers can click the abandon button only on old patches that are
definitely never going to make it in. This approach has the advantage that
it is easier for contributors to find changes and fix them up, even if the
change is very old.

3) Manually abandon after N months/weeks changes that have a -1 that was
never responded to

```
If a change is submitted and given a -1, and subsequently the author
becomes unresponsive for a few weeks, reviewers should leave reminder
comments on the review or attempt to contact the original author via IRC or
email. If the change is easy to fix, anyone should feel welcome to check
out the change and resubmit it using the same change ID to preserve
original authorship. If the author is unresponsive for at least 3 months
and no one else takes over the patch, core reviewers can abandon the patch,
leaving a detailed note about how the change can be restored.

If a change is submitted and given a -2, or it otherwise becomes clear that
the change can not make it in (for example, if an alternate change was
chosen to solve the problem), and the author has been unresponsive for at
least 3 months, a core reviewer should abandon the change.
```

Core reviewers can click the abandon button on changes that no one has
shown an interest in in N months/weeks, leaving a message about how to
restore the change if the author wants to come back to it. Puppet Labs does
this for its module pull requests, setting N at 1 month.

4) Auto-abandon after N months/weeks if patch has a -1 or -2

```
If a change is given a -2 and the author has been unresponsive for at least
3 months, a script will automatically abandon the change, leaving a message
about how the author can restore the change and attempt to resolve the -2
with the reviewer who left it.
```

We would use a tool like this one[1] to automatically abandon changes
meeting a certain criteria. We would have to decide whether we want to only
auto-abandon changes with -2's or go as far as to auto-abandon those with
-1's. The policy proposal above assumes -2. The tool would leave a canned
message about how to restore the change.


Option 1 has the problem of leaving clutter around, which the discussion
today seeks to solve.

Option 3 leaves the possibility that a change that is mostly good becomes
abandoned, making it harder for someone to find and restore it.

 I don't think option 4 is necessary because there are not an overwhelming
number of old changes (I count 9 that are currently over six months old).
In working through old changes a few months ago I found that many of them
are easy to fix up to remove a -1, and auto-abandoning removes the ability
for a human to make that call. Moreover, if a patch has a procedural -2
that ought to be lifted after some point, auto-abandonment has the
potential to accidentally throw out a change that was intended to be kept
(though presumably the core reviewer who left the -2 would notice the
abandonment and restore it if that was the case).

I am in favor of option 2. I think setting N to be 3 months or 6 months is
appropriate. I don't have very strong feelings about options 1 or 3. I'm
against option 4.

Colleen

[1]
https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh
__
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