On 23/08/12 05:22, Jeremy Baron wrote: I actually like it. If "Evil approval bot" mailed you warning that it will merge 12 pending changesets in two days if there's no action from your part, that would force some promptly action by you. I recently had a trivial patch to operations/puppet waiting for more than a month. When I noticed I hadn't added any Reviewer, and added to it, the changeset was fixed in the same day. But that also shows that nobody looked for new changes there. I have also seen people approving their own commits to core, something I'm not comfortable with. We could add some rules like: "If someone with approval to core has only positive votes, he can approve it after 1 week. If there was no review at all, he can approve it after 2 weeks". (Push repositories would obviously still allow auto-approval, and we could add exceptions for eg. translation changes) I was also recently unhappy when I discovered that one patch I thought I had open, had been abandoned without explanation. There can be good reasons for doing that, this is a bad idea, no longer needed, fixed in a different way in I123456... or even "closing because it has been waiting for a new patch for too long and I don't like seeing this open" (which I suspect was the case), but *please*, if you're closing another people's patch, leave an explanation!

On 23/08/12 15:07, Daniel Friesen wrote: I think we had such support in CodeReview (or at least considered adding it). Having to find out / magically know who to add as a reviewer is a gerrit drawback.

This looks good to me. -Subbu.

Not a chance in hell we'll ever allow this for the operations repo or the mediawiki deployment branches. That's because ops tends to be swamped with requests constantly. We work mostly on an interruption model. I doubt we'll change our workflow any time soon to check for changes that haven't asked for a review. I dislike this, very much. The workflow in ops requires this, but there's no reason it should happen in mediawiki core. Indeed. Like bugs, something should never be dropped without saying why. - Ryan

On 2012-08-23, at 2:42 AM, Siebrand Mazeland (WMF) wrote: Hey Siebrand, I prefer to keep the data collection as simple as possible. One way of fixing this issue is as follows: 1) Go to http://gerrit-stats.wmflabs.org/graphs/mediawiki/edit 2) Click on 'Options' 3) Click on 'Advanced' (right side of screen) 4) Click on 'rollPeriod' (bottom of screen, yellow box) 5) This allows to create a moving average, so you can replace the 1 with 7 meaning that each datapoint is the average of the past 7 days. This option applies to both metrics but it really smooths out the outliers. If you want to save this then please use another slug name (click on 'Info') and replace 'slug' and then click 'Save' else you will have changed Robla's original chart. Best, D

A real waste of time for reviewer is checking for code conventions which can be done automatically. Some static code analysis tools to automatically check conventions (spaces, missing messages['qqq']...) would help reduce the effort needed for review, allowing the human code reviewer to fully concentrate on real issues, and for the commiter to get almost immediate response for these small annoying convention issues. Is there a code analysis plugin that can be setup with jenkins? Eran Roz On Thu, Aug 23, 2012 at 11:44 AM, Niklas Laxström <niklas.laxstrom(a)gmail.com > On 23 August 2012 04:37, Rob Lanphier <robla(a)wikimedia.org> > > How is the process working for everyone? Is stuff getting reviewed > > quickly enough? How has it been for the reviewers out there? > > I'm doing a lot of code review[1], but I feel like it's wasting my > time by being inefficient. I'm still looking forward to diffs in > emails and/or one page diff view in Gerrit. Especially re-reviewing > big patches is just plain annoying when you have to go over all files > just to find the changes. > > My dashboard is mostly useless, it's full of -1 or +1 patches waiting > for an action by the submitter. Or +0 patches I gave +1 or -1 before > new patchset was submitted - they are indistinguishable from totally > new patches. It would also be nice to have different queues for things > like GSoC, Translate, sprint related, i18n review requested. > > It has also happened that my inline comments have been ignored because > a new patchset was submitted without addressing them, and subsequent > reviewers didn't notice the comments anymore. > > My own patches have been reviewed quickly, but that's because sprint > related patches are reviewed by other team members and non-sprint > related patches usually by Siebrand. There have been some exception > for patches that need review for someone outside of our team. > > [1] Since there are no statistics for this, I have no idea whether I'm > doing more or less than average, but I'm definitely spending a lot of > time on it. It's mainly the positive feedback I get that makes it > rewarding. > > -Niklas > > -- > Niklas Laxström > > _______________________________________________ > Wikitech-l mailing list > Wikitech-l(a)lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l >

Le 23/08/12 10:44, Niklas Laxström a écrit : I review big patches by fetching the change from the repository and using my local diff tool. Of course you need to use the Gerrit diff system to add inline comments. I am pretty sure Gerrit will come with saved search in the future. Chad will be able to confirm. As for the dashboard, just take care about lines which have no score from you (they should be bold) and do the -1 +1 score. Ignore the rest. If a patch was reset to +0, then it needs to be reviewed again by you (most probably a trivial review). It happened to me a few time to. One feature that could be able to Gerrit would be to prevent submitting a change till all inline comments have been replied to and marked as closed/fixed. <snip> -- Antoine "hashar" Musso

Slightly hijacking this thread but is related. I tried my hand at creating some gerrit related statistics: https://toolserver.org/~bawolff/gerrit-stats.htm Statistics might be the wrong word, its more the first 25 results of a saved search in reverse order - but also includes the total number of changesets with no review whatsoever, and some other things. There may be more things on it in the future. Last of all it includes a wall of shame, because I missed the old wall. This is still a rather rough version (As can be noted by lack of css or anything pretty). It also will update rather sporadically (I don't feel all that good about putting my private key on the toolserver to make this into a cron script on that server, so for now it is run by hand). Anyhow, feedback appreciated :) [Also source isn't public at the moment, but if anyone wants it let me know. It is a fairly hacky php script at the moment]

On Sat, Sep 1, 2012 at 6:17 AM, Bartosz Dziewoński &lt;matma.rex(a)gmail.com&gt; wrote: Sorry I was encoding the url the wrong way. Should be fixed now. No idea why it was broken in Opera and not Firefox --bawolff