I think our owner-suggesting tooling really needs to be better about deprioritizing owners who are on leave (as well as better load balancing). I'm not sure periodically removing and re-adding owners is the right long-term fix, but it could be acceptable if we're working on better infrastructure in the meanwhile.
I think another good, oft-discussed improvement would be to separate Review+1 from Owners+1. This would make the review load for owners more manageable, as they can just say that they approve of the overall change, without also marking every file they own as reviewed (and all that implies). On Thu, Jul 28, 2022 at 7:34 AM 'Matt Menke' via blink-dev < [email protected]> wrote: > > > On Thu, Jul 28, 2022 at 10:29 AM Ali Juma <[email protected]> wrote: > >> >> >> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara <[email protected]> >> wrote: >> >>> Thanks all for the input! >>> >>> Dana: >>> >>>> This list includes per-file owners, did the script look for 100 CLs in >>>> those files named by the rule when deciding to remove the person? >>> >>> >>> Thanks for pointing this out! I'll exclude per-file owners from the list >>> for now. >>> >>> Peter: >>> >>>> I'm worried that this process excludes/penalizes folks who may be OOO >>>> for extended leave (incl long stretches of parental leave, bereavement) and >>>> have that in their Gerrit status. This should not be a source of review >>>> latency, if it is Gerrit should better surface that they are OOO. >>>> Are any of the inactive owners, who did opt out last time, a source of >>>> review latency? I.e. are reviews assigned to them but they don't review >>>> them within some SLO window? Otherwise I strongly suggest we let folks >>>> decline the OWNERS removal (at other OWNERS' discretion who should probably >>>> review removal CLs). >>> >>> >>> I think Glen covered this topic very well. As written in this guideline >>> <https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#expectations-of-owners>, >>> owners are expected to be an active contributor to the directory ("Have the >>> bandwidth to contribute to reviews in a timely manner. ... Don't try to >>> discourage people from sending reviews, including writing “slow” or >>> “emeritus” after your name."). If you are on an extended leave and removed >>> by this process, you can explain it and re-add yourself through the owner >>> nomination process. Will it work? >>> >> >> The next guideline >> <https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#removal-of-owners> >> (on >> removal of owners) explicitly excludes owners who are on leave. I don't >> think we should be adding additional friction for folks who go on leave; >> the default assumption should be that when they return, they are just as >> capable of being a good owner as when they left, without them having to >> re-nominate themselves. >> > > The assumption is not that they're no longer good owners, but that people > shouldn't have to spend a week waiting on them to reply to a review before > giving up and trying someone else. Note that owners do not need to be > nominated - no one is losing their committer status. > > >> >> >>> >>> Matt: >>> >>>> Maybe it would make more sense to identify OWNERS who are not active >>>> globally in chrome/, instead of owners not active in a particular >>>> directory? How common are OWNERS active in Chrome, but high latency only >>>> for specific directories? >>> >>> >>> My personal opinion is that owners who made no contributions globally in >>> the past 6 months *or* owners who made no contribution to the directory >>> they own while there were 100+ commits in the past 6 months can be >>> identified as inactive owners. >>> >>> Note that this is not an irreversible process. When you have a reason, >>> you can explain it and re-add yourself through the owner nomination process. >>> >> > I'm not concerned about the fact that it's not reversible, but wasting > time. I received ~10 emails from the last mass owners purge, and Not > LGTMing a bunch of CLs created by a tool following completely opaque rules > seemed not a productive use of time. > > >> >>> I'm asking as someone who was recently inundated by auto-generated >>>> removal CLs, the majority of which did not make sense (admittedly, I >>>> believe it wasn't based on activity). The tool even seemed to want to >>>> remove all owners from some directories. >>> >>> >>> Right, the removal tool is not looking at activities, and this proposed >>> process is different from it. FWIW when I removed ~500 inactive owners last >>> year, it ended up removing (only) ~10 OWNERS files. So removing all owners >>> from some directories will be rare. >>> >>> Pavel: >>> >>>> The data in the table seems off, what is considered a "review": is that >>>> a "Code Review +1" or is that any review comment? >>> >>> >>> "Code Review +1" in the git commit log is considered as "review". >>> >>> >>>> I also have an edge case where I'm mostly interested in several files >>>> in a folder where other files are being changed more frequently, should I >>>> be optimizing OWNERS to list myself as per-file? >>> >>> >>> This sounds reasonable to me :) >>> >>> Glen: >>> >>>> I recently tried a similar automated audit of inactive owners - I >>>> looked for anyone who hadn't reviewed or authored a CL in 12 months >>>> anywhere, regardless of activity in the directory and found (as list, >>>> Google internal only) many accounts that no longer exist (or perhaps never >>>> did) in OWNERS. It probably has different false positives than the proposed >>>> set above. Maybe the intersection of the two sets would be sensible? >>> >>> >>> I'm happy to tweak the criteria depending on the conclusion of this >>> email thread :) >>> >>> >>> On Thu, Jul 28, 2022 at 3:32 PM Glen Robertson <[email protected]> >>> wrote: >>> >>>> > Having your name on OWNERS is an award for your previous amazing >>>> contributions >>>> I'm concerned that being in OWNERS is regarded as a reward, and being >>>> removed as a penalty -- it is part of the problem with cleaning up inactive >>>> OWNERS. I'd much prefer to have a separate "amazing contributors" file to >>>> list people who have made amazing contributions, without this affecting the >>>> code review process. >>>> >>>> Owners are supposed to be >>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#expectations-of-owners> >>>> active reviewers for a directory. I'd even expect us to remove people who >>>> go on long leave, unless Gerrit can understand that status and avoid >>>> suggesting them for reviews (currently it does not do that well). Re-adding >>>> an owner is not an arduous process, but adding days to a code review is a >>>> significant cost. >>>> >>>> I recently tried a similar automated audit of inactive owners - I >>>> looked for anyone who hadn't reviewed or authored a CL in 12 months >>>> anywhere, regardless of activity in the directory and found >>>> <https://chromium-review.googlesource.com/c/chromium/src/+/3750667> (as >>>> list, Google internal only >>>> <https://docs.google.com/spreadsheets/d/1BWvj44vJUjSXUHI85fJwX8AgIoYR5SkAm378pp80ljw/edit?resourcekey=0-7pInBE4h65x3c5t6Af1hbA#gid=0>) >>>> many accounts that no longer exist (or perhaps never did) in OWNERS. It >>>> probably has different false positives than the proposed set above. Maybe >>>> the intersection of the two sets would be sensible? >>>> >>>> On Thu, 28 Jul 2022 at 07:45, Pavel Feldman <[email protected]> >>>> wrote: >>>> >>>>> The data in the table seems off, what is considered a "review": is >>>>> that a "Code Review +1" or is that any review comment? >>>>> I also have an edge case where I'm mostly interested in several files >>>>> in a folder where other files are being changed more frequently, should I >>>>> be optimizing OWNERS to list myself as per-file? >>>>> >>>>> On Wednesday, July 27, 2022 at 2:16:47 PM UTC-7 Matt Menke wrote: >>>>> >>>>>> Maybe it would make more sense to identify OWNERS who are not active >>>>>> globally in chrome/, instead of owners not active in a particular >>>>>> directory? How common are OWNERS active in Chrome, but high latency only >>>>>> for specific directories? I'm asking as someone who was recently >>>>>> inundated >>>>>> by auto-generated removal CLs, the majority of which did not make sense >>>>>> (admittedly, I believe it wasn't based on activity). The tool even >>>>>> seemed >>>>>> to want to remove all owners from some directories. >>>>>> >>>>>> On Wednesday, July 27, 2022 at 5:03:05 PM UTC-4 [email protected] >>>>>> wrote: >>>>>> >>>>>>> I echo Dana's concern about removing per-file owners and would like >>>>>>> to see that policy rethought. Agree with Peter's observations as well. >>>>>>> >>>>>>> -Ken >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, Jul 27, 2022 at 9:12 AM Peter Boström <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> I'm worried that this process excludes/penalizes folks who may be >>>>>>>> OOO for extended leave (incl long stretches of parental leave, >>>>>>>> bereavement) >>>>>>>> and have that in their Gerrit status. This should not be a source of >>>>>>>> review >>>>>>>> latency, if it is Gerrit should better surface that they are OOO. >>>>>>>> >>>>>>>> Are any of the inactive owners, who did opt out last time, a source >>>>>>>> of review latency? I.e. are reviews assigned to them but they don't >>>>>>>> review >>>>>>>> them within some SLO window? Otherwise I strongly suggest we let folks >>>>>>>> decline the OWNERS removal (at other OWNERS' discretion who should >>>>>>>> probably >>>>>>>> review removal CLs). >>>>>>>> >>>>>>>> On Wed, Jul 27, 2022 at 8:08 AM <[email protected]> wrote: >>>>>>>> >>>>>>> This list includes per-file owners, did the script look for 100 CLs >>>>>>>>> in *those files* named by the rule when deciding to remove the >>>>>>>>> person? >>>>>>>>> >>>>>>>>> On Tue, Jul 26, 2022 at 9:16 PM Kentaro Hara <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> As of 2022 July, Chromium has 4531 OWNERS files containing 6850 >>>>>>>>>> names. These include inactive owners, which are one of the sources >>>>>>>>>> of slow >>>>>>>>>> code review latency. One year ago, we cleaned up inactive owners >>>>>>>>>> <https://groups.google.com/a/chromium.org/g/chromium-dev/c/MpOgk56qKS0/m/HHy7G19oAwAJ> >>>>>>>>>> and removed ~500 inactive owners. I propose running the clean-up >>>>>>>>>> process >>>>>>>>>> again to keep the OWNERS files updated. >>>>>>>>>> >>>>>>>>>> Specifically, a person is identified as an "inactive" owner iff: >>>>>>>>>> >>>>>>>>>> - >>>>>>>>>> >>>>>>>>>> The person didn't commit or review any CLs in the directory >>>>>>>>>> they own while there were 100+ CLs that touched the directory in >>>>>>>>>> the past 6 >>>>>>>>>> months (as of July 6, 2022). >>>>>>>>>> >>>>>>>>>> Last year, I gave the inactive owners an option to flip the >>>>>>>>>> decision manually to stay as an owner, but for this cycle, I'm >>>>>>>>>> planning to >>>>>>>>>> remove the inactive owners unconditionally. The rationale is 1) if >>>>>>>>>> the >>>>>>>>>> person made no contribution on a very active directory for 6 months, >>>>>>>>>> it >>>>>>>>>> will be reasonable to say that the person is inactive, and 2) if >>>>>>>>>> there is >>>>>>>>>> any special reason for it and the person needs to stay as an owner, >>>>>>>>>> the >>>>>>>>>> person can show evidence that they are meeting the owners >>>>>>>>>> expectations >>>>>>>>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#expectations-of-owners> >>>>>>>>>> and be readded through the standard OWNERS nomination process. >>>>>>>>>> >>>>>>>>>> Specifically, people listed in this spreadsheet >>>>>>>>>> <https://docs.google.com/spreadsheets/d/1gJbXzTaoITvCDmQaqMmGCvfOngrcFtMPmMsGhHgEV_4/edit#gid=0> >>>>>>>>>> are identified as inactive owners and will be removed. >>>>>>>>>> >>>>>>>>>> I understand this is a tricky proposal. Having your name on >>>>>>>>>> OWNERS is an award for your previous amazing contributions, and I >>>>>>>>>> understand your feeling about your name being removed. However, I >>>>>>>>>> think >>>>>>>>>> it's important to keep the OWNERS files updated so that Chromium >>>>>>>>>> developers >>>>>>>>>> can find active owners and improve the code review latency. >>>>>>>>>> >>>>>>>>>> If you have any questions / concerns, please let me know. Thanks! >>>>>>>>>> -- >>>>>>>>>> Kentaro Hara, Tokyo >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> You received this message because you are subscribed to the >>>>>>>>>> Google Groups "blink-dev" group. >>>>>>>>>> >>>>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>>>>> send an email to [email protected]. >>>>>>>>> >>>>>>>>> >>>>>>>>>> To view this discussion on the web visit >>>>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com >>>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>>>> . >>>>>>>>>> >>>>>>>>> -- >>>>>>>>> -- >>>>>>>>> Chromium Developers mailing list: [email protected] >>>>>>>> >>>>>>>> >>>>>>>>> View archives, change email options, or unsubscribe: >>>>>>>>> http://groups.google.com/a/chromium.org/group/chromium-dev >>>>>>>>> --- >>>>>>>>> You received this message because you are subscribed to the Google >>>>>>>>> Groups "Chromium-dev" group. >>>>>>>>> >>>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>>>> send an email to [email protected]. >>>>>>>> >>>>>>>> >>>>>>>>> To view this discussion on the web visit >>>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHtyhaTNC4tgQbqbUq%2BQdFfcORr3aFobjgbeE%2BTaVf7eDgU2Bg%40mail.gmail.com >>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHtyhaTNC4tgQbqbUq%2BQdFfcORr3aFobjgbeE%2BTaVf7eDgU2Bg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>>> . >>>>>>>>> >>>>>>>> -- >>>>>>>> -- >>>>>>>> Chromium Developers mailing list: [email protected] >>>>>>> >>>>>>> >>>>>>>> View archives, change email options, or unsubscribe: >>>>>>>> http://groups.google.com/a/chromium.org/group/chromium-dev >>>>>>>> --- >>>>>>>> You received this message because you are subscribed to the Google >>>>>>>> Groups "Chromium-dev" group. >>>>>>>> >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>>> send an email to [email protected]. >>>>>>> >>>>>>> >>>>>>>> To view this discussion on the web visit >>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAGFX3sFB9G8R2MyHT6rjVtEFRAKMeyCTH6Yu0DYqUOfLPCxCBw%40mail.gmail.com >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAGFX3sFB9G8R2MyHT6rjVtEFRAKMeyCTH6Yu0DYqUOfLPCxCBw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>> . >>>>>>>> >>>>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "blink-dev" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/0a2a01e2-652b-4e31-895c-f020e7b46358n%40chromium.org >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/0a2a01e2-652b-4e31-895c-f020e7b46358n%40chromium.org?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> >>> >>> -- >>> Kentaro Hara, Tokyo >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "blink-dev" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyCNN1%3DpfT%3DCWPmc4%2Bi9PmGs-%3DbX9e2mUi2bHthF%2B0w-w%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyCNN1%3DpfT%3DCWPmc4%2Bi9PmGs-%3DbX9e2mUi2bHthF%2B0w-w%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEK7mvr5vjo0FdbYepkOLBDGJN7KLivmauf4G-HQ1rf67f7pSQ%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEK7mvr5vjo0FdbYepkOLBDGJN7KLivmauf4G-HQ1rf67f7pSQ%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACwGi-50bAUKX0TMMuGqBqG-HeF0b1fmEDTowDcYLabUgcB2zw%40mail.gmail.com.
