Brion, i would love to use gerrit more fully (that is until we finally migrate! :)), but gerrit to my knowledge does not differentiate between a CC (review if you want to) and TO (i want you to +2). Having multiple cooks means some patches don't get merged at all. I feel each patch should be assigned to a person who will +2 it. This does not preclude others from +2ing it, but it designates one responsible for the answer.
On Thu, Jan 29, 2015 at 2:33 PM, Brion Vibber <bvib...@wikimedia.org> wrote: > I'd like us to start by using the review request system already in gerrit > more fully. > > Personally I've got a bunch of incoming reviews in my queue where I'm not > sure the current status of them or if it's wise to land them. :) > > First step is probably to go through the existing old patches in > everybody's queues and either do the review, abandon the patch, or trim > down reviewers who aren't familiar with the code area. > > Rejected patches should be abandoned to get them out of the queues. > > Then we should go through unassigned patches more aggressively... > > We also need to make sure we have default reviewers for modules, which will > be relevant also to triaging bug reports. > > -- brion > On Jan 29, 2015 2:03 PM, "Yuri Astrakhan" <yastrak...@wikimedia.org> > wrote: > > > How about a simple script to create a phabricator task after a few days > (a > > week?) of a patch inactivity to review that patch. It will allow "assign > > to", allow managers to see each dev's review queue, and will prevent > > patches to fall through the cracks. > > > > Obviously this won't be needed after we move gerrit to phabricator. > > > > On Thu, Jan 29, 2015 at 1:44 PM, James Douglas <jdoug...@wikimedia.org> > > wrote: > > > > > This is a situation where disciplined testing can come in really handy. > > > > > > If I submit a patch, and the patch passes the tests that have been > > > specified for the feature it implements (or the bug it fixes), and the > > code > > > coverage is sufficiently high, then a reviewer has a running start in > > terms > > > of confidence in the correctness and completeness of the patch. > > > > > > Practically speaking, this doesn't necessarily rely on rest of the > > project > > > already having a very level of code coverage; as long as there are > tests > > > laid out for the feature in question, and the patch makes those tests > > pass, > > > it gives the code reviewer a real shot in the arm. > > > > > > On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson <jdlrob...@gmail.com> > wrote: > > > > > > > Thanks for kicking off the conversation Brad :-) > > > > > > > > Just mean at the moment. I hacked together and I'm more than happy to > > > > iterate on this and improve the reporting. > > > > > > > > On the subject of patch abandonment: Personally I think we should be > > > > abandoning inactive patches. They cause unnecessary confusion to > > > > someone coming into a new extension wanting to help out. We may want > > > > to change the name to 'abandon' to 'remove from code review queue' as > > > > abandon sounds rather nasty and that's not at all what it actually > > > > does - any abandoned patch can be restored at any time if necessary. > > > > > > > > > > > > On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) > > > > <bjor...@wikimedia.org> wrote: > > > > > On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson <jdlrob...@gmail.com> > > > > wrote: > > > > > > > > > >> The average time for code to go from submitted to merged appears > to > > be > > > > >> 29 days over a dataset of 524 patches, excluding all that were > > written > > > > >> by the L10n bot. There is a patchset there that has been _open_ > for > > > > >> 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 > > PM > > > > >> is -1ed by me and needs a rebase. > > > > >> > > > > > > > > > > Mean or median? > > > > > > > > > > I recall talk a while back about someone else (Quim, I think?) > doing > > > this > > > > > same sort of analysis, and considering the same issues over patches > > > that > > > > > seem to have been abandoned by their author and so on, which led to > > > > > discussions of whether we should go around abandoning patches that > > have > > > > > been -1ed for a long time, etc. Without proper consideration of > those > > > > sorts > > > > > of issues, the statistics don't seem particularly useful. > > > > > > > > > > > > > > > -- > > > > > Brad Jorsch (Anomie) > > > > > Software Engineer > > > > > Wikimedia Foundation > > > > > _______________________________________________ > > > > > Wikitech-l mailing list > > > > > Wikitech-l@lists.wikimedia.org > > > > > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > > > > > > > > > > > > > > > > -- > > > > Jon Robson > > > > * http://jonrobson.me.uk > > > > * https://www.facebook.com/jonrobson > > > > * @rakugojon > > > > > > > > _______________________________________________ > > > > 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 > _______________________________________________ > 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
