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 <bvibber(a)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" <yastrakhan(a)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 <jdouglas(a)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 <jdlrobson(a)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)
> > <bjorsch(a)wikimedia.org> wrote:
> > > On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson <jdlrobson(a)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(a)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(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
Wikitech-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
Wikitech-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
Wikitech-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l