On Fri, Jul 28, 2017 at 9:54 AM, Gijs Kruitbosch via governance < governance@lists.mozilla.org> wrote:
> (x-post platform + governance + bmo, please followup-to governance) > > Today I was asked for super-review. Although I've been around a while, I > am not a super-reviewer. I can't remember the last time before today that I > was asked (it's quite possibly: never). The person who asked me was mostly > just wanting me to do a second review, in addition to the review the patch > in question had already had. > > I looked for our policy of super-review and list of current super > reviewers ( https://www.mozilla.org/en-US/about/governance/policies/revi > ewers/ ). > > It seems the list is woefully out of date (it lists gavin, shaver, biesi, > and a number of other people who I'm confident are wonderful but either > haven't used bugzilla for several years, or have only used it to un-cc > themselves from issues or similar non-super-review-y activity). > > In public bugs > 1300000 (filed in the last 11 months; I can't get the > 'field changed after' bugzilla advanced query to work right), there have > been 13 super-reviews. 9 in Core, 2 in NSS and 1 each in Android Background > Services and MailNews. (link: https://mzl.la/2w6Ttba ) > > From a quick browse of these bugs, as best I can tell the only reason the > superreview flag was used was to indicate "I want a second review from this > other person who I know knows this code well", and often it was omitted on > the checkin comment or included as if it were a normal review (ie r=foo,bar > rather than r=foo,sr=bar), even where the people concerned *are* on the > superreview list (which, equally often, they're not). > > I note here that you can easily request multiple reviews (or any other > flag) in bugzilla or mozreview by simply comma-separating the reviewers (or > their uniquely-matching aliases) in the single textbox for that flag i.e. > ":mary,:john" will request review from those 2 people. > > For something that happens on the order of 100 times less often than > "normal" review requests, I don't think the extra field, documentation etc. > is worth the confusion. > > > I therefore propose 2 things: > 1) we remove the super-review flag from Core/Firefox/Toolkit, or perhaps > everything except maybe NSS (on the assumption it is actively used there). > 2) we either remove or in some obvious way mark the above-linked > super-review document as out of date / archived / historical, and remove > any links such as may exist from documentation/mdn that point to that page. > > If there is a vibrant culture of super-review that matches up to the > afore-linked document that I am completely unaware of because I move in the > 'wrong' circles, and that somehow wasn't captured in my bugzilla query, > please bring that up. If that were the case, please can we update the > document to list the Right People, and hide the super-review field in > products and components where it isn't routinely used (which I am > reasonably confident would still include the Firefox and Toolkit products) > to avoid confusion. > The topic of "do we need super-review" has come up before: * https://groups.google.com/d/msg/mozilla.dev.planning/fZV-DYnqQEc/KWJIDWJZ4ZMJ * https://groups.google.com/d/msg/mozilla.dev.platform/xwYLRuPQVnw/SwA7JM2lTK4J Given that enough people seem to question its utility and given the low frequency of use, I lean towards retiring it. Plus with the transition to Phabricator, everything changes. I doubt the Phabricator team has any inclination to formalize the concept of super-review in that tool. _______________________________________________ governance mailing list governance@lists.mozilla.org https://lists.mozilla.org/listinfo/governance
