+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 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.
>
>
> __________________________________________________________________________
> 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
>
>


-- 
Duncan Thomas

__________________________________________________________________________
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

Reply via email to