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?
On Apr 9, 2014 12:08 PM, "Quim Gil" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org 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
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" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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 jdlrobson@gmail.com 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" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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 liangent@gmail.com 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 jdlrobson@gmail.com 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" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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" liangent@gmail.com 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 jdlrobson@gmail.com 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" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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_copyAl...
Lets follow up on bug 41074 mentioned above.
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 hashar+wmf@free.fr 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_copyAl...
Lets follow up on bug 41074 mentioned above.
-- Antoine "hashar" Musso
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 9 April 2014 09:12, Jon Robson jdlrobson@gmail.com 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,...
… 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,...
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.
Quim Gil qgil@wikimedia.org 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:
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
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
Thank you for the quick and useful round of feedback.
On Wednesday, April 9, 2014, Tim Landscheidt tim@tim-landscheidt.de 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
Tim Landscheidt tim@tim-landscheidt.de wrote:
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.
As somebody who contributes something in development surges (like a week per three months or so), I think that cleaning up statistics to make our code review process look nicer is not the way to go.
What about automatically accepting the change that has not been reviewed by anybody for two weeks or so instead?
I agree that -1 is practically a death penalty to a change. But that's not a positive development, because even a mild -1's completety discourages anybody to post a positive review (I wonder how many +1 or neutral comments were posted *after* some of the WMF reviewers posted a -1).
Some examples from my own dashboard:
1) https://gerrit.wikimedia.org/r/#/c/99068/
practically dead although I completely disagree with the -1 reviewer, as reflected in the comment afterwards.
2) https://gerrit.wikimedia.org/r/#/c/11562/
My favourite -1 here is "needs rebase".
In general our review process disourages somehow incremental updating of the patches (do we know how many non-original-submitters posted follow up patchsets, not comments?).
This kind of review discourages refactoring or some non-trivial changes. See "seems too complex" in the example #2 above.
Regarding Openstack policies: I'd say we should not follow them.
I used to be #2 git-review contributor according to launchpad until recently. I gave up mainly because of my inability to propose some larger change to this relatively simple script. For a nice example of this, please see
https://review.openstack.org/#/c/5720/
I have given up to contribute to this project some time after this, I have no time to play politics to submit a set of tiny changes and play the rebase game depending on the random order they might would have got reviewed.
The next time I find time to improve Johnny the causual developer experience with gerrit I will just rewrite git-review from scratch. The amount of the red tape openstack-infra has built around their projects is simply not justifiable for such a simple utility like git-review. Time will tell if gerrit-based projects generally fare better than others.
//Saper
On Sun, Apr 13, 2014 at 1:37 PM, Marcin Cieslak saper@saper.info wrote:
My favourite -1 here is "needs rebase".
Well, obviously trivial rebases should be done automatically by the system (which OpenStack's system does), and changes that need a rebase due to conflicts should be fixed. Reviewer time is generally in short supply, so it makes sense to have the committer do any conflict resolution.
Regarding Openstack policies: I'd say we should not follow them.
I used to be #2 git-review contributor according to launchpad until recently. I gave up mainly because of my inability to propose some larger change to this relatively simple script. For a nice example of this, please see
https://review.openstack.org/#/c/5720/
I have given up to contribute to this project some time after this, I have no time to play politics to submit a set of tiny changes and play the rebase game depending on the random order they might would have got reviewed.
This seems like an odd change to use as an example. There seems to be no politics in play there. All of the reviews were encouraging, but it looked like there was a release happening during your reviews and it caused a number of merge conflicts. The change was automatically abandoned, but you could have restored it and pinged someone on the infra team.
The next time I find time to improve Johnny the causual developer experience with gerrit I will just rewrite git-review from scratch. The amount of the red tape openstack-infra has built around their projects is simply not justifiable for such a simple utility like git-review. Time will tell if gerrit-based projects generally fare better than others.
Maybe, until you start looking at their statistics:
< http://stackalytics.com/?release=icehouse&metric=marks&project_type=...
If you notice, this release cycle they had 1,200 reviewers. One organization had 20k reviews over the cycle, and the top 5 each had over 10k reviews. Their process scales way better than Wikimedia's, but that's also due to the way projects are split up and organized as well.
- Ryan
On Sun, Apr 13, 2014 at 1:37 PM, Marcin Cieslak saper@saper.info wrote:
I agree that -1 is practically a death penalty to a change. But that's not a positive development, because even a mild -1's completety discourages anybody to post a positive review (I wonder how many +1 or neutral comments were posted *after* some of the WMF reviewers posted a -1).
That seems dramatic and hasn't been my experience. A -1 does mean others will be less likely to review, but it doesn't discourage positive comments about the change. Often people will -1 a beneficial change because they see how it can be improved further, but the answer is to respond to their comment with "I've filed bug NNNNN for that suggestion", and invite them to remove the -1.
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.
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" qgil@wikimedia.org 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 Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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.
On Wed, Apr 9, 2014 at 12:53 PM, Antoine Musso hashar+wmf@free.fr 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
Hi,
On Wed, Apr 09, 2014 at 09:19:59AM -0700, Chad wrote:
No. I've thought the OpenStack policy is rude to contributors.
Me too. I'd also prefer we do not automatically abandon changes.
Best regards, Christian
Please don't do this for operations/puppet. We've had patches sitting there for way longer than this that still got improved and merged eventually.
On Thu, Apr 10, 2014 at 1:26 AM, Christian Aistleitner < christian@quelltextlich.at> wrote:
Hi,
On Wed, Apr 09, 2014 at 09:19:59AM -0700, Chad wrote:
No. I've thought the OpenStack policy is rude to contributors.
Me too. I'd also prefer we do not automatically abandon changes.
Best regards, Christian
-- ---- quelltextlich e.U. ---- \ ---- Christian Aistleitner ---- Companies' registry: 360296y in Linz Christian Aistleitner Gruendbergstrasze 65a Email: christian@quelltextlich.at 4040 Linz, Austria Phone: +43 732 / 26 95 63 Fax: +43 732 / 26 95 63 Homepage: http://quelltextlich.at/
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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.
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
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?
On Wed, 09 Apr 2014 22:07:09 +0200, Bartosz Dziewoński matma.rex@gmail.com 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.)
Bartosz Dziewoński matma.rex@gmail.com wrote:
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?
What are the benefits?
If someone is overwhelmed by changes in -1 in his dash- board(s), it sounds odd that they would want to exclude only changes older than three months when they could exclude all of them.
Tim
On 9 April 2014 14:10, Tim Landscheidt tim@tim-landscheidt.de wrote:
If someone is overwhelmed by changes in -1 in his dash- board(s), it sounds odd that they would want to exclude only changes older than three months when they could exclude all of them.
"-age:3m" will achieve this too, of course.
J.
On 04/09/2014 06:25 PM, James Forrester wrote:
On 9 April 2014 14:10, Tim Landscheidt tim@tim-landscheidt.de wrote:
If someone is overwhelmed by changes in -1 in his dash- board(s), it sounds odd that they would want to exclude only changes older than three months when they could exclude all of them.
"-age:3m" will achieve this too, of course.
J.
James reminds me:
https://gerrit.wikimedia.org/r/Documentation/user-dashboards.html
https://gerrit.wikimedia.org/r/Documentation/user-search.html
It sucks to want to review code but be discouraged by mountains of changesets that you don't find relevant. Bookmarking the URLs of some super-selective searches or dashboards will probably help you out; check out the docs and swap tips.
wikitech-l@lists.wikimedia.org