Abandoning -1 code reviews automatically?

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Abandoning -1 code reviews automatically?

Quim Gil-2
In relation to our discussion about code review metrics at
http://korma.wmflabs.org/browser/gerrit_review_queue.html

I just learned that OpenStack has a policy to abandon automatically reviews
sitting in -1 during more than one week:

https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1

Maybe one week is too tight for the reality of our project, but what about
2-4 weeks?

Brad and others said last week that they were not interested in code review
queue metrics mixing pending -1 reviews. Maybe the solution is not to tweak
the metrics, but to effectively abandon those changesets automatically?


--
Quim Gil
Engineering Community Manager @ Wikimedia Foundation
http://www.mediawiki.org/wiki/User:Qgil
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Brian Wolff
On Apr 9, 2014 12:08 PM, "Quim Gil" <[hidden email]> wrote:
>
> In relation to our discussion about code review metrics at
> http://korma.wmflabs.org/browser/gerrit_review_queue.html
>
> I just learned that OpenStack has a policy to abandon automatically
reviews
> sitting in -1 during more than one week:
>
> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
>
> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?
>
> Brad and others said last week that they were not interested in code
review
> queue metrics mixing pending -1 reviews. Maybe the solution is not to
tweak

> the metrics, but to effectively abandon those changesets automatically?
>
>
> --
> Quim Gil
> Engineering Community Manager @ Wikimedia Foundation
> http://www.mediawiki.org/wiki/User:Qgil
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Id be fine with that, but can we start very conservitively, say 3 months?
Just to gauge reaction. If things go well then we could figure out a more
aggresive auto-abandon schedule.

Id also say we should only auto-abandon -1's. -2's sometimes mean something
weird is going on that often have nothing to do with the committer.

--bawolff
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Jon Robson
In reply to this post by Quim Gil-2
Yes yes yes please!

I've been manually doing this in mobile with a short friendly note saying
if the owner wants to resubmit it they can feel free to at a later date. My
gerrit is just a spam queue right now.

Just to clarify - if someone submits a patch then says 1 month later via
comment I still want to work on it do we abandon it for the time being or
keep it open?
On 9 Apr 2014 08:08, "Quim Gil" <[hidden email]> wrote:

> In relation to our discussion about code review metrics at
> http://korma.wmflabs.org/browser/gerrit_review_queue.html
>
> I just learned that OpenStack has a policy to abandon automatically reviews
> sitting in -1 during more than one week:
>
> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
>
> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?
>
> Brad and others said last week that they were not interested in code review
> queue metrics mixing pending -1 reviews. Maybe the solution is not to tweak
> the metrics, but to effectively abandon those changesets automatically?
>
>
> --
> Quim Gil
> Engineering Community Manager @ Wikimedia Foundation
> http://www.mediawiki.org/wiki/User:Qgil
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

liangent
I have some changesets where I uploaded a reworked patchset several weeks
or even months after an original -1 was given...

Anyway a changeset can be easily "renewed" by rebasing it, but the side
effect is that all existing -1's get flushed. If people start to use this
method to extend the period, it's not something good...


On Wed, Apr 9, 2014 at 11:25 PM, Jon Robson <[hidden email]> wrote:

> Yes yes yes please!
>
> I've been manually doing this in mobile with a short friendly note saying
> if the owner wants to resubmit it they can feel free to at a later date. My
> gerrit is just a spam queue right now.
>
> Just to clarify - if someone submits a patch then says 1 month later via
> comment I still want to work on it do we abandon it for the time being or
> keep it open?
> On 9 Apr 2014 08:08, "Quim Gil" <[hidden email]> wrote:
>
> > In relation to our discussion about code review metrics at
> > http://korma.wmflabs.org/browser/gerrit_review_queue.html
> >
> > I just learned that OpenStack has a policy to abandon automatically
> reviews
> > sitting in -1 during more than one week:
> >
> > https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
> >
> > Maybe one week is too tight for the reality of our project, but what
> about
> > 2-4 weeks?
> >
> > Brad and others said last week that they were not interested in code
> review
> > queue metrics mixing pending -1 reviews. Maybe the solution is not to
> tweak
> > the metrics, but to effectively abandon those changesets automatically?
> >
> >
> > --
> > Quim Gil
> > Engineering Community Manager @ Wikimedia Foundation
> > http://www.mediawiki.org/wiki/User:Qgil
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

nischay nahata
Some of my changes sitting there are a few month old.

Maybe removing such changes first will reduce the -1 changes to a greater
extent and help us get a clearer idea?


On Wed, Apr 9, 2014 at 9:09 PM, Liangent <[hidden email]> wrote:

> I have some changesets where I uploaded a reworked patchset several weeks
> or even months after an original -1 was given...
>
> Anyway a changeset can be easily "renewed" by rebasing it, but the side
> effect is that all existing -1's get flushed. If people start to use this
> method to extend the period, it's not something good...
>
>
> On Wed, Apr 9, 2014 at 11:25 PM, Jon Robson <[hidden email]> wrote:
>
> > Yes yes yes please!
> >
> > I've been manually doing this in mobile with a short friendly note saying
> > if the owner wants to resubmit it they can feel free to at a later date.
> My
> > gerrit is just a spam queue right now.
> >
> > Just to clarify - if someone submits a patch then says 1 month later via
> > comment I still want to work on it do we abandon it for the time being or
> > keep it open?
> > On 9 Apr 2014 08:08, "Quim Gil" <[hidden email]> wrote:
> >
> > > In relation to our discussion about code review metrics at
> > > http://korma.wmflabs.org/browser/gerrit_review_queue.html
> > >
> > > I just learned that OpenStack has a policy to abandon automatically
> > reviews
> > > sitting in -1 during more than one week:
> > >
> > > https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
> > >
> > > Maybe one week is too tight for the reality of our project, but what
> > about
> > > 2-4 weeks?
> > >
> > > Brad and others said last week that they were not interested in code
> > review
> > > queue metrics mixing pending -1 reviews. Maybe the solution is not to
> > tweak
> > > the metrics, but to effectively abandon those changesets automatically?
> > >
> > >
> > > --
> > > Quim Gil
> > > Engineering Community Manager @ Wikimedia Foundation
> > > http://www.mediawiki.org/wiki/User:Qgil
> > > _______________________________________________
> > > Wikitech-l mailing list
> > > [hidden email]
> > > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>



--
Cheers,

Nischay Nahata
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Brian Wolff
In reply to this post by liangent
If we do do this, I think we should make it very clear to users that they
can unabandon a change if they plan to fix it.

Personally id like to get rid of the changesets that have been open over a
year by someone who hasnt done anything mediawiki related for months and
probably isnt coming back.

--bawolff

On Apr 9, 2014 12:39 PM, "Liangent" <[hidden email]> wrote:

>
> I have some changesets where I uploaded a reworked patchset several weeks
> or even months after an original -1 was given...
>
> Anyway a changeset can be easily "renewed" by rebasing it, but the side
> effect is that all existing -1's get flushed. If people start to use this
> method to extend the period, it's not something good...
>
>
> On Wed, Apr 9, 2014 at 11:25 PM, Jon Robson <[hidden email]> wrote:
>
> > Yes yes yes please!
> >
> > I've been manually doing this in mobile with a short friendly note
saying
> > if the owner wants to resubmit it they can feel free to at a later
date. My
> > gerrit is just a spam queue right now.
> >
> > Just to clarify - if someone submits a patch then says 1 month later via
> > comment I still want to work on it do we abandon it for the time being
or

> > keep it open?
> > On 9 Apr 2014 08:08, "Quim Gil" <[hidden email]> wrote:
> >
> > > In relation to our discussion about code review metrics at
> > > http://korma.wmflabs.org/browser/gerrit_review_queue.html
> > >
> > > I just learned that OpenStack has a policy to abandon automatically
> > reviews
> > > sitting in -1 during more than one week:
> > >
> > > https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
> > >
> > > Maybe one week is too tight for the reality of our project, but what
> > about
> > > 2-4 weeks?
> > >
> > > Brad and others said last week that they were not interested in code
> > review
> > > queue metrics mixing pending -1 reviews. Maybe the solution is not to
> > tweak
> > > the metrics, but to effectively abandon those changesets
automatically?

> > >
> > >
> > > --
> > > Quim Gil
> > > Engineering Community Manager @ Wikimedia Foundation
> > > http://www.mediawiki.org/wiki/User:Qgil
> > > _______________________________________________
> > > Wikitech-l mailing list
> > > [hidden email]
> > > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Tim Landscheidt
In reply to this post by Quim Gil-2
Quim Gil <[hidden email]> wrote:

> In relation to our discussion about code review metrics at
> http://korma.wmflabs.org/browser/gerrit_review_queue.html

> I just learned that OpenStack has a policy to abandon automatically reviews
> sitting in -1 during more than one week:

> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1

> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?

> Brad and others said last week that they were not interested in code review
> queue metrics mixing pending -1 reviews. Maybe the solution is not to tweak
> the metrics, but to effectively abandon those changesets automatically?

No.  First of all, this would give anyone who has -1 and can
click some buttons the power to abandon changes or at least
whip a contributor into action: "Fix this /now/, or else!"
I don't think this would be a healthy atmosphere for devel-
opment.  From my limited view of development at OpenStack,
it appears that doesn't force contributors to produce +2able
changes in a jiffy, but just give up.

Second, changes with -1 appear in the Gerrit UI by default,
while abandoned changes do not.  So all the work that was
done is effectively lost when finally someone new comes
along and wants to tackle a problem that has been approached
before.

Third, this sends out the message: "We welcome you as a con-
tributor!  A patch, how awesome!  Oh, sitting more than four
weeks?  Then go away and only come back when your code is
ready because you are messing up our statistics!"

I'm all for abandoning changes when the author doesn't react
and the patch doesn't apply anymore (not in a technical
sense, but the patch's concept cannot be rebased to the cur-
rent HEAD).  But forcing work on many just so that a metric
can be easier calculated by one is putting the burden on the
wrong side.

Tim


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Antoine Musso-3
In reply to this post by liangent
Le 09/04/2014 17:39, Liangent a écrit :
> I have some changesets where I uploaded a reworked patchset several weeks
> or even months after an original -1 was given...
>
> Anyway a changeset can be easily "renewed" by rebasing it, but the side
> effect is that all existing -1's get flushed. If people start to use this
> method to extend the period, it's not something good...

OpenStack have a script which reapply all votes when a trivial rebase is
detected.

Our bug is https://bugzilla.wikimedia.org/show_bug.cgi?id=41074

Apparently we can now confiure labels to recopy on trivial rebase or
when no code change (ie only commit summary is changed)

Doc:

https://gerrit.wikimedia.org/r/Documentation/config-labels.html#label_copyAllScoresOnTrivialRebase

Lets follow up on bug 41074 mentioned above.

--
Antoine "hashar" Musso


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Antoine Musso-3
In reply to this post by Quim Gil-2
Le 09/04/2014 17:08, Quim Gil a écrit :

> In relation to our discussion about code review metrics at
> http://korma.wmflabs.org/browser/gerrit_review_queue.html
>
> I just learned that OpenStack has a policy to abandon automatically reviews
> sitting in -1 during more than one week:
>
> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
>
> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?

Hello,

I am fine having changes abandoned automatically after a couple weeks.
Just make sure registered users are allowed to restore changes though.

My questions for the general audience are why we do keep those patches
around.

Are we expecting the author to somehow come back, look at their Gerrit
dashboard and resume it?  Thus letting patch open as a convenience.

Are we expecting someone else to take over the patch and complete it?
The inactive patches could thus been seen as a list of features to have,
but then we have Bugzilla for that :]

So yeah, dropping automatically seems fine to me.

--
Antoine "hashar" Musso


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Jon Robson
In reply to this post by Antoine Musso-3
From a personal perspective, the fact we have so much code to review
in the Gerrit code review queue makes it harder for __me__ to
prioritise the important patchsets and thus review them. I would
hazard a guess that such a system would help work out which patches
are important and which are just nice to haves/controversial and would
actually lead to a more healthy code review system.

If something has a -1 or -2 it needs work. No arguments. Either work
on it or abandon it and come back to it later.

As Brian says patches can be resubmitted, abandoning is a bit of a
strong word, it just removes them from the review queue making it
easier for other people's code to get review.

I think we should try this approach, see how it works, collect
feedback from developers and revisit it in a month to see if the
situation is even better.




On Wed, Apr 9, 2014 at 9:04 AM, Antoine Musso <[hidden email]> wrote:

> Le 09/04/2014 17:39, Liangent a écrit :
>> I have some changesets where I uploaded a reworked patchset several weeks
>> or even months after an original -1 was given...
>>
>> Anyway a changeset can be easily "renewed" by rebasing it, but the side
>> effect is that all existing -1's get flushed. If people start to use this
>> method to extend the period, it's not something good...
>
> OpenStack have a script which reapply all votes when a trivial rebase is
> detected.
>
> Our bug is https://bugzilla.wikimedia.org/show_bug.cgi?id=41074
>
> Apparently we can now confiure labels to recopy on trivial rebase or
> when no code change (ie only commit summary is changed)
>
> Doc:
>
> https://gerrit.wikimedia.org/r/Documentation/config-labels.html#label_copyAllScoresOnTrivialRebase
>
> Lets follow up on bug 41074 mentioned above.
>
> --
> Antoine "hashar" Musso
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l



--
Jon Robson
* http://jonrobson.me.uk
* https://www.facebook.com/jonrobson
* @rakugojon

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Brian Wolff
In reply to this post by Tim Landscheidt
>
> No.  First of all, this would give anyone who has -1 and can
> click some buttons the power to abandon changes or at least
> whip a contributor into action: "Fix this /now/, or else!"
> I don't think this would be a healthy atmosphere for devel-
> opment.  From my limited view of development at OpenStack,
> it appears that doesn't force contributors to produce +2able
> changes in a jiffy, but just give up.
>
> Second, changes with -1 appear in the Gerrit UI by default,
> while abandoned changes do not.  So all the work that was
> done is effectively lost when finally someone new comes
> along and wants to tackle a problem that has been approached
> before.

Its essentially impossible to find anything useful in the gerrit ui
between the mess of ignored -1 patches. I think the visibility of both
types of changes are pretty low.

>
> Third, this sends out the message: "We welcome you as a con-
> tributor!  A patch, how awesome!  Oh, sitting more than four
> weeks?  Then go away and only come back when your code is
> ready because you are messing up our statistics!"
>
> I'm all for abandoning changes when the author doesn't react
> and the patch doesn't apply anymore (not in a technical
> sense, but the patch's concept cannot be rebased to the cur-
> rent HEAD).  But forcing work on many just so that a metric
> can be easier calculated by one is putting the burden on the
> wrong side.
>
> Tim
>

I do agree with this. I don't think we should let statistical needs
dictate community practise. However, I would like a (conservative)
auto-abandon thing for other reasons.

--bawolff

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Chad
In reply to this post by Quim Gil-2
No. I've thought the OpenStack policy is rude to contributors.

If your personal review queues are too spammy you should be more aggressive
about removing *yourself* from the change.

-Chad

-Chad
On Apr 9, 2014 8:08 AM, "Quim Gil" <[hidden email]> wrote:

> In relation to our discussion about code review metrics at
> http://korma.wmflabs.org/browser/gerrit_review_queue.html
>
> I just learned that OpenStack has a policy to abandon automatically reviews
> sitting in -1 during more than one week:
>
> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
>
> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?
>
> Brad and others said last week that they were not interested in code review
> queue metrics mixing pending -1 reviews. Maybe the solution is not to tweak
> the metrics, but to effectively abandon those changesets automatically?
>
>
> --
> Quim Gil
> Engineering Community Manager @ Wikimedia Foundation
> http://www.mediawiki.org/wiki/User:Qgil
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Mark Holmquist-2
In reply to this post by Quim Gil-2
On Wed, Apr 09, 2014 at 08:08:01AM -0700, Quim Gil wrote:
> I just learned that OpenStack has a policy to abandon automatically reviews
> sitting in -1 during more than one week:
>
> https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c1
>
> Maybe one week is too tight for the reality of our project, but what about
> 2-4 weeks?

As probably one of the worst offenders, I'm not a fan.

I've written and subsequently left patchsets to sit for many months -
possibly over a year in some cases - but I don't think any of them should
be abandoned.

I see an abandoned patchset as an admittance by the person who wrote the
patch that they never should have and that the feature or bugfix was so
wrong, it could not even be fixed up. Most of the patches I've left sitting
for a while aren't that bad.

Even if you don't subscribe to that view, leaving -1'd patches around
isn't so terrible because they're a start along the right path. Rather
than having someone else start on the same project in a totally new patch,
because they didn't see the open change for the same purpose, I'd like
them to take up the patch I started that is slightly flawed and finish
it up. Abandoning patches will discourage this sort of behaviour. Even
without auto-abandoning patches I have seen people work on things that
I have already started on...

If we do wind up doing this, I foresee a lot more sleepless nights in
my near future trying to catch up with all of the code review that has
accumulated over the past 2 years during which I often got a few days
or weeks to hack on a project and then couldn't come back to it for a
while.

I really like sleep, guys. Let's leave things as they are.

--
Mark Holmquist
Software Engineer, Multimedia
Wikimedia Foundation
[hidden email]
https://wikimediafoundation.org/wiki/User:MHolmquist

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

James Forrester-4
In reply to this post by Jon Robson
On 9 April 2014 09:12, Jon Robson <[hidden email]> wrote:

> From a personal perspective, the fact we have so much code to review
> in the Gerrit code review queue makes it harder for __me__ to
> prioritise the important patchsets and thus review them.


​For which repos? Do those repos have a proper dashboard set up, or are
they just using the default one?

For example, in VisualEditor, the default "Recent Changes" dashboard is
pretty useless:

https://gerrit.wikimedia.org/r/#/projects/mediawiki/extensions/VisualEditor,dashboards/default:recent


​ whereas the custom multi-part one we have is much more useful at
prioritising, especially cross-repo issues:

https://gerrit.wikimedia.org/r/#/projects/mediawiki/extensions/VisualEditor,dashboards/custom:custom​

I can imagine that for e.g. the Mobile and Multimedia teams, who have a
bunch of different repos in which they work, that this might be
particularly helpful. ​I'd be happy to help teams and interested groups
build ones for other repos; mediawiki/core.git might be a bit of a
challenge, but worth attempting.


​On the wider point, I think it would be a mistake to automatically abandon
-1'ed or -2'ed patches; as Chad says, it's a pretty negative way to
interact with volunteers, and as Mark says, old patches are useful to keep
around as a way of reminding yourself (and team members) of non-urgent but
important work (like adding a new CI test and passing it, a gargantuan task
at times).

​J.
--
James D. Forrester
Product Manager, VisualEditor
Wikimedia Foundation, Inc.

[hidden email] | @jdforrester
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Federico Leva (Nemo)
In reply to this post by Mark Holmquist-2
Currently a commit is "Abandoned" when rejected, mostly. If we abandon
valid patches just because they're imperfect, we'll need a way to list
the abandoned patches we'd welcome work on. Therefore, I think the user
pressing the "abandon" button for a stale change should first be
required to file a bug for it.

Nemo

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Quim Gil-2
In reply to this post by Tim Landscheidt
Thank you for the quick and useful round of feedback.

On Wednesday, April 9, 2014, Tim Landscheidt <[hidden email]> wrote:

> forcing work on many just so that a metric
> can be easier calculated by one is putting the burden on the
> wrong side.


Just to be clear, I asked the question wondering whether automatically
abandoning apparently abandoned changes would improve our process.

Following with Brad's point, and regardless of the outcome of this
discussion, we will try to remove the -1 open reviews of the graphs showing
data about changesets that require attention from reviewers.

https://bugzilla.wikimedia.org/show_bug.cgi?id=63533#c5


--
Quim Gil
Engineering Community Manager @ Wikimedia Foundation
http://www.mediawiki.org/wiki/User:Qgil
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Antoine Musso-3
In reply to this post by Chad
Le 09/04/2014 18:19, Chad a écrit :
> No. I've thought the OpenStack policy is rude to contributors.
>
> If your personal review queues are too spammy you should be more aggressive
> about removing *yourself* from the change.


I disagree without you that it is rude. I find them warmly welcoming new
contributors even when they definitely lack python skill (my case a few
months ago, that has improved thanks to their reviews).

OpenStack is slightly different.

- They have a policy to have each patch to be approved by two different
people. Which mean most of the code would usually at least get one
review when some of our code barely has one author.

- I believe most OpenStack contributors are professional software
developers being paid to contribute to OpenStack.  That is their duty.
Whereas we have a tons of volunteers (which is a good thing) which cant
always afford to reply to all the reviews in a timely manner or sometime
have no clue what they are doing (no offense there).

- They also use http://status.openstack.org/reviews/ , that ranks
patches per project and priority of the bug attached to it.

- They have WIP to let people open a change that is never abandoned.
Much like a Gerrit draft but public :)

We also host a bunch of repositories that most senior folks have no
interest in.  Our collections of extensions can probably be cleaned up
and the one for which we have no interest (as a community) should be
hosted elsewhere.

Finally, removing yourself from changes would not prevent you from
seeing the bit rotting changes when you list all open changes for a set
of repositories.  On integration/* I keep them open as TODO item and
there is only a handful of them so that remains manageable.

I would not mind participating in a triage to clean up the oldest
patches or at least contact the various authors and ask them to continue
or abandon their patches.  That could even be automatized, and
abandoning automatically with a nice message would have the same effect.

Hey, we can even replace Abandon with Expire if that sounds less rude.

--
Antoine "hashar" Musso


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Chad
On Wed, Apr 9, 2014 at 12:53 PM, Antoine Musso <[hidden email]> wrote:

> Le 09/04/2014 18:19, Chad a écrit :
> > No. I've thought the OpenStack policy is rude to contributors.
> >
> > If your personal review queues are too spammy you should be more
> aggressive
> > about removing *yourself* from the change.
>
>
> I disagree without you that it is rude. I find them warmly welcoming new
> contributors even when they definitely lack python skill (my case a few
> months ago, that has improved thanks to their reviews).
>
>
I'm not saying OpenStack is rude, I'm sure they're all nice folks.
I just find their abandonment policy rude :)


> Finally, removing yourself from changes would not prevent you from
> seeing the bit rotting changes when you list all open changes for a set
> of repositories.  On integration/* I keep them open as TODO item and
> there is only a handful of them so that remains manageable.
>
>
Per above, blindly searching for open changes isn't very useful. This is
why we have dashboards.


> Hey, we can even replace Abandon with Expire if that sounds less rude.
>

We're not doing custom builds of Gerrit anymore. This would fall under that.

-Chad
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Bartosz Dziewoński
In reply to this post by Quim Gil-2
I think we totally should do this, but with a really long expiration period (3 months suggested by Brian seem nice).

The concerns raised by a few people here are valid, but I think they could be offset with some simple improvements:

* Never re-abandon a patchset that has been previously restored (note that I mean patchset, not changeset).
* Send a notice (a comment) on the patch that it will be abandoned, explaining why we do this and how to prevent it.

How about this?

--
Matma Rex

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Abandoning -1 code reviews automatically?

Bartosz Dziewoński
On Wed, 09 Apr 2014 22:07:09 +0200, Bartosz Dziewoński <[hidden email]> wrote:

> * Send a notice (a comment) on the patch that it will be abandoned, explaining why we do this and how to prevent it.

(Of course I mean sending it before the actual abandonment, e.g. a month before the proposed three-month inactivity period ends.)

--
Matma Rex

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
12