Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

['Kentaro Hara' via blink-dev]

Hi

Thanks all for the thoughts! I discussed with some of the people on this
thread and Eng Review offline.

It's important to keep OWNERS files up-dated. OWNERS should include people
who are meeting the owner's expectations

("Have
committed or reviewed substantial work to the affected directory within the
last ninety days", "Have the bandwidth to contribute to reviews in a timely
manner"). The consumers of OWNERS shouldn't need to care what's happening
with individuals on that list, and should be able to get useful, fast
turnaround time from them. I get the argument that we shouldn't penalize
folks on leaves, however, we also have to make sure that the system doesn't
punish everybody by having a number of absent owners, so I'm still in favor
of removing absent owners and we'd just add them back in when they're back
and active again.

On the other hand, I agree with the feedback that removing inactive owners
unconditionally is too aggressive (thanks for the feedback!). So I propose
going with the following plan:

1) Add this policy
 to
code_reviews.md. The policy says that if you are removed while you were on
a leave, you can re-add yourself when you are back.
2) Use the (already existing) automated owner removal tool and let it
create a CL to *remove owners who made 0 commits / reviews to Chromium in
the past 12 months (2021/7/26 - 2022/7/26)*. This will be a strong signal
to indicate inactive owners. There are 320 owners

.
3) The CL adds existing owners of the directory (including the owner to be
removed) to a reviewer list. The CL description clearly asks the reviewers
to check that the removed owner is not on a leave. The CL is committed
after getting an LGTM from one of the reviewers.
4) If an owner is mistakenly removed while they are on a leave (which is
not likely to happen due to 3)), they can re-add themselves according to the
policy .

It's possible that some owners made 0 commits / reviews to Chromium in the
past 12 months because their directories are under maintenance mode and
didn't have many CLs. In this case, they (or other owners in the directory)
can say Code-Review -1 in 3).

If you have any concerns, please let me know.

Thanks!



On Fri, Aug 5, 2022 at 2:01 PM Andy Perelson  wrote:

> Hi folks,
>
> A fair bit of the feedback on Kentaro's proposal has focused on improving
> Owners suggestions and better handling OOO Owners. We do believe there's a
> lot of room to improve owners suggestions to help improve review time. OOO
> is one aspect, but you could also imagine such a system taking into account
> time zones, review loads, etc. It's an area we'd like to improve on but
> also a complicated one. I can't promise specific features or a timeline at
> this stage, but know that we hear and agree with the feedback that this
> would be a useful area to improve that could positively help review times.
> And one we've been thinking about for a while.
>
> Thanks,
>
> Andy
> On behalf of the Chrome Ops Source team
>
> On Thu, Aug 4, 2022 at 5:43 PM Jayson Adams  wrote:
>
>> This policy, despite the proposed disclaimer paragraph, still feels like
>> it penalizes people who are on leave. Whether it's parental or disability
>> or any other kind of leave, no one should have to jump through hoops upon
>> their return to restart their job (or I guess why not also remove
>> committer status, in the interest of making things tidy, while you're at
>> it?).
>>
>> Given that your updated list only contains 47 owners, it seems like
>> checking each one to see if they're on leave (and removing them if so)
>> would not be that big of an issue.
>>
>> On Mon, Aug 1, 2022 at 6:57 AM Kentaro Hara  wrote:
>>
>>> Discussed with Peter and Glen offline.
>>>
>>> I excluded per-file owners and updated the spreadsheet
>>> 
>>> (based on the data as of July 26 when I announced this).
>>>
>>> To mitigate the concern about removing long-time OOO owners and making
>>> them feel uncomfortable about re-adding themselves when they are back, we
>>> decided to add the following paragraph
>>>  to
>>> the owners guideline (which is under review):
>>>
>>> "For the purpose of not slowing down code review latency, Chromium
>>> removes inactive owners (e.g., those who made no contributions in the past
>>> 6 months) on a regular basis. The script does not take into account
>>> personal situations like a long leave and thus can be wrong. If you are
>>> removed by the automated process for a wrong reason while you are 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Andy Perelson]

Hi folks,

A fair bit of the feedback on Kentaro's proposal has focused on improving
Owners suggestions and better handling OOO Owners. We do believe there's a
lot of room to improve owners suggestions to help improve review time. OOO
is one aspect, but you could also imagine such a system taking into account
time zones, review loads, etc. It's an area we'd like to improve on but
also a complicated one. I can't promise specific features or a timeline at
this stage, but know that we hear and agree with the feedback that this
would be a useful area to improve that could positively help review times.
And one we've been thinking about for a while.

Thanks,

Andy
On behalf of the Chrome Ops Source team

On Thu, Aug 4, 2022 at 5:43 PM Jayson Adams  wrote:

> This policy, despite the proposed disclaimer paragraph, still feels like
> it penalizes people who are on leave. Whether it's parental or disability
> or any other kind of leave, no one should have to jump through hoops upon
> their return to restart their job (or I guess why not also remove
> committer status, in the interest of making things tidy, while you're at
> it?).
>
> Given that your updated list only contains 47 owners, it seems like
> checking each one to see if they're on leave (and removing them if so)
> would not be that big of an issue.
>
> On Mon, Aug 1, 2022 at 6:57 AM Kentaro Hara  wrote:
>
>> Discussed with Peter and Glen offline.
>>
>> I excluded per-file owners and updated the spreadsheet
>> 
>> (based on the data as of July 26 when I announced this).
>>
>> To mitigate the concern about removing long-time OOO owners and making
>> them feel uncomfortable about re-adding themselves when they are back, we
>> decided to add the following paragraph
>>  to
>> the owners guideline (which is under review):
>>
>> "For the purpose of not slowing down code review latency, Chromium
>> removes inactive owners (e.g., those who made no contributions in the past
>> 6 months) on a regular basis. The script does not take into account
>> personal situations like a long leave and thus can be wrong. If you are
>> removed by the automated process for a wrong reason while you are being a
>> good owner, you can create a CL to re-add yourself to OWNERS and land after
>> getting local owner's approval. You just need to explain the reason (e.g.,
>> long leave) and don't need to make a substantial contribution to become an
>> owner again."
>>
>> Manuel:
>>
>>> Just for completeness, there are some OWNERS files that require a
>>> nomination process. At least third_party/blink/renderer/core/OWNERS
>>> requires that.
>>
>>
>> I hope the above process mitigates the concern.
>>
>> 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?
>>
>>
>> FWIW I created a list
>> 
>> of OWNERS who made no commits / reviews in the past 6 months (as of July
>> 26). We could take an intersection between this list and my proposed list
>> but I'd prefer simply going with my proposed list with the above mitigation
>> process.
>>
>> Peter:
>>
>>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>>> bypass local owner approval for this (i.e. land it unconditionally). If
>>> some of them are owners in very active folders and have had a lot of
>>> reviews assigned to them, sure. This is about 1% of ownership entries if
>>> I'm understanding this correctly. How badly are they contributing to review
>>> latency in order to override "and they are not on a leave during that time"
>>> as a policy? I like this policy, and it's supposed to still be the policy.
>>
>>
>> We are removing (only) 85 owners (47 owners after per-file owners are
>> excluded) because we removed ~500 owners one year ago. I think it's
>> important to run this cleanup process periodically to keep the codebase
>> up-dated. Our Chrome infra team is now brainstorming an idea to automate
>> the process of removing inactive owners.
>>
>>
>> I'm planning to execute the cleanup next week. If you have any questions,
>> please let me know. Thanks!
>>
>>
>>
>> On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:
>>
>>> I specifically disagree with the "for this cycle, I'm planning to remove
>>> the inactive owners unconditionally" bit. If you suggest this removal but
>>> you defer to local OWNERS to make the call I think that's fine to send out
>>> removal CLs for. I would encourage you and/or the reviewers to look at the
>>> removed owner's calendar if they're internal and confirm that they've not
>>> been on long-term leave.
>>>
>>> I 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Gabriel Charette]

On Mon, Aug 1, 2022 at 11:22 AM 'Christian Dullweber' via Chromium-dev <
chromium-...@chromium.org> wrote:

> I looked into the first 10 entries manually and there are multiple cases
> where I don't think it is correct to remove them.
>
> There are people who are on long term leave and are indicating this in
> their status message.
> I would be quite annoyed if I had to figure out all the files I was
> owner for and restore this when I come back from a leave.
> Could you check for each person you want to remove if the status message
> has an indication that the OWNER is only temporarily inactive?
>
> There are also otherwise active owners who have a comment that limits
> their ownership to a subset of files.
> I wonder why it would be useful to remove people who are otherwise active
> just because they haven't reviewed files in a certain directory?
>
> There are also per-file owners but I guess the list just isn't updated yet?
>
> As an alternative to implementing the suggestions above:
> How about sending CLs with autosubmit enabled for each removal to all
> owners of the corresponding file and see if they agree with the removal or
> not?
>

We used to have documentation that required that all large-scale changes be
`git cl split` unless 100% mechanical (Owners-Override), I can't find it
anymore (lost in Style Guide / LSC documentation move to .MD?). It sounds
like this isn't 100% mechanical and it thus makes a lot of sense to me that
it'd at least require a local owner's approval.


>
>
>
> On Mon, 1 Aug 2022 at 15:56, Kentaro Hara  wrote:
>
>> Discussed with Peter and Glen offline.
>>
>> I excluded per-file owners and updated the spreadsheet
>> 
>> (based on the data as of July 26 when I announced this).
>>
>> To mitigate the concern about removing long-time OOO owners and making
>> them feel uncomfortable about re-adding themselves when they are back, we
>> decided to add the following paragraph
>>  to
>> the owners guideline (which is under review):
>>
>> "For the purpose of not slowing down code review latency, Chromium
>> removes inactive owners (e.g., those who made no contributions in the past
>> 6 months) on a regular basis. The script does not take into account
>> personal situations like a long leave and thus can be wrong. If you are
>> removed by the automated process for a wrong reason while you are being a
>> good owner, you can create a CL to re-add yourself to OWNERS and land after
>> getting local owner's approval. You just need to explain the reason (e.g.,
>> long leave) and don't need to make a substantial contribution to become an
>> owner again."
>>
>> Manuel:
>>
>>> Just for completeness, there are some OWNERS files that require a
>>> nomination process. At least third_party/blink/renderer/core/OWNERS
>>> requires that.
>>
>>
>> I hope the above process mitigates the concern.
>>
>> 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?
>>
>>
>> FWIW I created a list
>> 
>> of OWNERS who made no commits / reviews in the past 6 months (as of July
>> 26). We could take an intersection between this list and my proposed list
>> but I'd prefer simply going with my proposed list with the above mitigation
>> process.
>>
>> Peter:
>>
>>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>>> bypass local owner approval for this (i.e. land it unconditionally). If
>>> some of them are owners in very active folders and have had a lot of
>>> reviews assigned to them, sure. This is about 1% of ownership entries if
>>> I'm understanding this correctly. How badly are they contributing to review
>>> latency in order to override "and they are not on a leave during that time"
>>> as a policy? I like this policy, and it's supposed to still be the policy.
>>
>>
>> We are removing (only) 85 owners (47 owners after per-file owners are
>> excluded) because we removed ~500 owners one year ago. I think it's
>> important to run this cleanup process periodically to keep the codebase
>> up-dated. Our Chrome infra team is now brainstorming an idea to automate
>> the process of removing inactive owners.
>>
>>
>> I'm planning to execute the cleanup next week. If you have any questions,
>> please let me know. Thanks!
>>
>>
>>
>> On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:
>>
>>> I specifically disagree with the "for this cycle, I'm planning to remove
>>> the inactive owners unconditionally" bit. If you suggest this removal but
>>> you defer to local OWNERS to make the call I think that's fine to send out
>>> 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Jayson Adams]

This policy, despite the proposed disclaimer paragraph, still feels like it
penalizes people who are on leave. Whether it's parental or disability or
any other kind of leave, no one should have to jump through hoops upon
their return to restart their job (or I guess why not also remove
committer status, in the interest of making things tidy, while you're at
it?).

Given that your updated list only contains 47 owners, it seems like
checking each one to see if they're on leave (and removing them if so)
would not be that big of an issue.

On Mon, Aug 1, 2022 at 6:57 AM Kentaro Hara  wrote:

> Discussed with Peter and Glen offline.
>
> I excluded per-file owners and updated the spreadsheet
> 
> (based on the data as of July 26 when I announced this).
>
> To mitigate the concern about removing long-time OOO owners and making
> them feel uncomfortable about re-adding themselves when they are back, we
> decided to add the following paragraph
>  to
> the owners guideline (which is under review):
>
> "For the purpose of not slowing down code review latency, Chromium removes
> inactive owners (e.g., those who made no contributions in the past 6
> months) on a regular basis. The script does not take into account personal
> situations like a long leave and thus can be wrong. If you are removed by
> the automated process for a wrong reason while you are being a good owner,
> you can create a CL to re-add yourself to OWNERS and land after getting
> local owner's approval. You just need to explain the reason (e.g., long
> leave) and don't need to make a substantial contribution to become an owner
> again."
>
> Manuel:
>
>> Just for completeness, there are some OWNERS files that require a
>> nomination process. At least third_party/blink/renderer/core/OWNERS
>> requires that.
>
>
> I hope the above process mitigates the concern.
>
> 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?
>
>
> FWIW I created a list
> 
> of OWNERS who made no commits / reviews in the past 6 months (as of July
> 26). We could take an intersection between this list and my proposed list
> but I'd prefer simply going with my proposed list with the above mitigation
> process.
>
> Peter:
>
>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>> bypass local owner approval for this (i.e. land it unconditionally). If
>> some of them are owners in very active folders and have had a lot of
>> reviews assigned to them, sure. This is about 1% of ownership entries if
>> I'm understanding this correctly. How badly are they contributing to review
>> latency in order to override "and they are not on a leave during that time"
>> as a policy? I like this policy, and it's supposed to still be the policy.
>
>
> We are removing (only) 85 owners (47 owners after per-file owners are
> excluded) because we removed ~500 owners one year ago. I think it's
> important to run this cleanup process periodically to keep the codebase
> up-dated. Our Chrome infra team is now brainstorming an idea to automate
> the process of removing inactive owners.
>
>
> I'm planning to execute the cleanup next week. If you have any questions,
> please let me know. Thanks!
>
>
>
> On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:
>
>> I specifically disagree with the "for this cycle, I'm planning to remove
>> the inactive owners unconditionally" bit. If you suggest this removal but
>> you defer to local OWNERS to make the call I think that's fine to send out
>> removal CLs for. I would encourage you and/or the reviewers to look at the
>> removed owner's calendar if they're internal and confirm that they've not
>> been on long-term leave.
>>
>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>> bypass local owner approval for this (i.e. land it unconditionally). If
>> some of them are owners in very active folders and have had a lot of
>> reviews assigned to them, sure. This is about 1% of ownership entries if
>> I'm understanding this correctly. How badly are they contributing to review
>> latency in order to override "and they are not on a leave during that time"
>> as a policy? I like this policy, and it's supposed to still be the policy.
>>
>> If any of these OWNERS entries had a significant amount of reviews
>> assigned to them during this period (i.e. sorta-evidence of latency), you
>> could use that as additional justification in the CLs to get the OWNERS to
>> approve the (possibly temporary) removal.
>>
>> For fixing review latency which is the real goal 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

['Christian Dullweber' via blink-dev]

I looked into the first 10 entries manually and there are multiple cases
where I don't think it is correct to remove them.

There are people who are on long term leave and are indicating this in
their status message.
I would be quite annoyed if I had to figure out all the files I was
owner for and restore this when I come back from a leave.
Could you check for each person you want to remove if the status message
has an indication that the OWNER is only temporarily inactive?

There are also otherwise active owners who have a comment that limits their
ownership to a subset of files.
I wonder why it would be useful to remove people who are otherwise active
just because they haven't reviewed files in a certain directory?

There are also per-file owners but I guess the list just isn't updated yet?

As an alternative to implementing the suggestions above:
How about sending CLs with autosubmit enabled for each removal to all
owners of the corresponding file and see if they agree with the removal or
not?



On Mon, 1 Aug 2022 at 15:56, Kentaro Hara  wrote:

> Discussed with Peter and Glen offline.
>
> I excluded per-file owners and updated the spreadsheet
> 
> (based on the data as of July 26 when I announced this).
>
> To mitigate the concern about removing long-time OOO owners and making
> them feel uncomfortable about re-adding themselves when they are back, we
> decided to add the following paragraph
>  to
> the owners guideline (which is under review):
>
> "For the purpose of not slowing down code review latency, Chromium removes
> inactive owners (e.g., those who made no contributions in the past 6
> months) on a regular basis. The script does not take into account personal
> situations like a long leave and thus can be wrong. If you are removed by
> the automated process for a wrong reason while you are being a good owner,
> you can create a CL to re-add yourself to OWNERS and land after getting
> local owner's approval. You just need to explain the reason (e.g., long
> leave) and don't need to make a substantial contribution to become an owner
> again."
>
> Manuel:
>
>> Just for completeness, there are some OWNERS files that require a
>> nomination process. At least third_party/blink/renderer/core/OWNERS
>> requires that.
>
>
> I hope the above process mitigates the concern.
>
> 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?
>
>
> FWIW I created a list
> 
> of OWNERS who made no commits / reviews in the past 6 months (as of July
> 26). We could take an intersection between this list and my proposed list
> but I'd prefer simply going with my proposed list with the above mitigation
> process.
>
> Peter:
>
>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>> bypass local owner approval for this (i.e. land it unconditionally). If
>> some of them are owners in very active folders and have had a lot of
>> reviews assigned to them, sure. This is about 1% of ownership entries if
>> I'm understanding this correctly. How badly are they contributing to review
>> latency in order to override "and they are not on a leave during that time"
>> as a policy? I like this policy, and it's supposed to still be the policy.
>
>
> We are removing (only) 85 owners (47 owners after per-file owners are
> excluded) because we removed ~500 owners one year ago. I think it's
> important to run this cleanup process periodically to keep the codebase
> up-dated. Our Chrome infra team is now brainstorming an idea to automate
> the process of removing inactive owners.
>
>
> I'm planning to execute the cleanup next week. If you have any questions,
> please let me know. Thanks!
>
>
>
> On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:
>
>> I specifically disagree with the "for this cycle, I'm planning to remove
>> the inactive owners unconditionally" bit. If you suggest this removal but
>> you defer to local OWNERS to make the call I think that's fine to send out
>> removal CLs for. I would encourage you and/or the reviewers to look at the
>> removed owner's calendar if they're internal and confirm that they've not
>> been on long-term leave.
>>
>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>> bypass local owner approval for this (i.e. land it unconditionally). If
>> some of them are owners in very active folders and have had a lot of
>> reviews assigned to them, sure. This is about 1% of ownership entries if
>> I'm understanding this correctly. How badly are they contributing to review
>> latency in order 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[K. Moon]

Given the amount of discussion this topic has generated, I almost wonder if
the, "Let's just run this script and fix any problems later," approach is
actually taking more time in total than something more nuanced.

Also, is there an AI for any team to follow up on some of the suggestions
about surfacing OOO status, or other sources of high review latency?

On Mon, Aug 1, 2022 at 8:22 AM Christian Dullweber 
wrote:

> I looked into the first 10 entries manually and there are multiple cases
> where I don't think it is correct to remove them.
>
> There are people who are on long term leave and are indicating this in
> their status message.
> I would be quite annoyed if I had to figure out all the files I was
> owner for and restore this when I come back from a leave.
> Could you check for each person you want to remove if the status message
> has an indication that the OWNER is only temporarily inactive?
>
> There are also otherwise active owners who have a comment that limits
> their ownership to a subset of files.
> I wonder why it would be useful to remove people who are otherwise active
> just because they haven't reviewed files in a certain directory?
>
> There are also per-file owners but I guess the list just isn't updated yet?
>
> As an alternative to implementing the suggestions above:
> How about sending CLs with autosubmit enabled for each removal to all
> owners of the corresponding file and see if they agree with the removal or
> not?
>
>
>
> On Mon, 1 Aug 2022 at 15:56, Kentaro Hara  wrote:
>
>> Discussed with Peter and Glen offline.
>>
>> I excluded per-file owners and updated the spreadsheet
>> 
>> (based on the data as of July 26 when I announced this).
>>
>> To mitigate the concern about removing long-time OOO owners and making
>> them feel uncomfortable about re-adding themselves when they are back, we
>> decided to add the following paragraph
>>  to
>> the owners guideline (which is under review):
>>
>> "For the purpose of not slowing down code review latency, Chromium
>> removes inactive owners (e.g., those who made no contributions in the past
>> 6 months) on a regular basis. The script does not take into account
>> personal situations like a long leave and thus can be wrong. If you are
>> removed by the automated process for a wrong reason while you are being a
>> good owner, you can create a CL to re-add yourself to OWNERS and land after
>> getting local owner's approval. You just need to explain the reason (e.g.,
>> long leave) and don't need to make a substantial contribution to become an
>> owner again."
>>
>> Manuel:
>>
>>> Just for completeness, there are some OWNERS files that require a
>>> nomination process. At least third_party/blink/renderer/core/OWNERS
>>> requires that.
>>
>>
>> I hope the above process mitigates the concern.
>>
>> 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?
>>
>>
>> FWIW I created a list
>> 
>> of OWNERS who made no commits / reviews in the past 6 months (as of July
>> 26). We could take an intersection between this list and my proposed list
>> but I'd prefer simply going with my proposed list with the above mitigation
>> process.
>>
>> Peter:
>>
>>> I don't think that 85/6850 OWNERS entries are a large enough problem to
>>> bypass local owner approval for this (i.e. land it unconditionally). If
>>> some of them are owners in very active folders and have had a lot of
>>> reviews assigned to them, sure. This is about 1% of ownership entries if
>>> I'm understanding this correctly. How badly are they contributing to review
>>> latency in order to override "and they are not on a leave during that time"
>>> as a policy? I like this policy, and it's supposed to still be the policy.
>>
>>
>> We are removing (only) 85 owners (47 owners after per-file owners are
>> excluded) because we removed ~500 owners one year ago. I think it's
>> important to run this cleanup process periodically to keep the codebase
>> up-dated. Our Chrome infra team is now brainstorming an idea to automate
>> the process of removing inactive owners.
>>
>>
>> I'm planning to execute the cleanup next week. If you have any questions,
>> please let me know. Thanks!
>>
>>
>>
>> On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:
>>
>>> I specifically disagree with the "for this cycle, I'm planning to remove
>>> the inactive owners unconditionally" bit. If you suggest this removal but
>>> you defer to local OWNERS to make the call I think that's fine to send out
>>> removal CLs for. I would encourage you 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Kentaro Hara]

Discussed with Peter and Glen offline.

I excluded per-file owners and updated the spreadsheet

(based on the data as of July 26 when I announced this).

To mitigate the concern about removing long-time OOO owners and making them
feel uncomfortable about re-adding themselves when they are back, we
decided to add the following paragraph
 to the
owners guideline (which is under review):

"For the purpose of not slowing down code review latency, Chromium removes
inactive owners (e.g., those who made no contributions in the past 6
months) on a regular basis. The script does not take into account personal
situations like a long leave and thus can be wrong. If you are removed by
the automated process for a wrong reason while you are being a good owner,
you can create a CL to re-add yourself to OWNERS and land after getting
local owner's approval. You just need to explain the reason (e.g., long
leave) and don't need to make a substantial contribution to become an owner
again."

Manuel:

> Just for completeness, there are some OWNERS files that require a
> nomination process. At least third_party/blink/renderer/core/OWNERS
> requires that.


I hope the above process mitigates the concern.

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?


FWIW I created a list

of OWNERS who made no commits / reviews in the past 6 months (as of July
26). We could take an intersection between this list and my proposed list
but I'd prefer simply going with my proposed list with the above mitigation
process.

Peter:

> I don't think that 85/6850 OWNERS entries are a large enough problem to
> bypass local owner approval for this (i.e. land it unconditionally). If
> some of them are owners in very active folders and have had a lot of
> reviews assigned to them, sure. This is about 1% of ownership entries if
> I'm understanding this correctly. How badly are they contributing to review
> latency in order to override "and they are not on a leave during that time"
> as a policy? I like this policy, and it's supposed to still be the policy.


We are removing (only) 85 owners (47 owners after per-file owners are
excluded) because we removed ~500 owners one year ago. I think it's
important to run this cleanup process periodically to keep the codebase
up-dated. Our Chrome infra team is now brainstorming an idea to automate
the process of removing inactive owners.


I'm planning to execute the cleanup next week. If you have any questions,
please let me know. Thanks!



On Sat, Jul 30, 2022 at 3:56 AM Peter Boström  wrote:

> I specifically disagree with the "for this cycle, I'm planning to remove
> the inactive owners unconditionally" bit. If you suggest this removal but
> you defer to local OWNERS to make the call I think that's fine to send out
> removal CLs for. I would encourage you and/or the reviewers to look at the
> removed owner's calendar if they're internal and confirm that they've not
> been on long-term leave.
>
> I don't think that 85/6850 OWNERS entries are a large enough problem to
> bypass local owner approval for this (i.e. land it unconditionally). If
> some of them are owners in very active folders and have had a lot of
> reviews assigned to them, sure. This is about 1% of ownership entries if
> I'm understanding this correctly. How badly are they contributing to review
> latency in order to override "and they are not on a leave during that time"
> as a policy? I like this policy, and it's supposed to still be the policy.
>
> If any of these OWNERS entries had a significant amount of reviews
> assigned to them during this period (i.e. sorta-evidence of latency), you
> could use that as additional justification in the CLs to get the OWNERS to
> approve the (possibly temporary) removal.
>
> For fixing review latency which is the real goal presumably, I think
> handling OOO and load balancing are both more important vectors here. We
> shouldn't need to get inactive owners down to 0 to solve review latency.
>
> On Thu, Jul 28, 2022 at 6:47 PM Kentaro Hara  wrote:
>
>> +1 to improving the tool to handle OOO owners.
>>
>> Let's step back. The problem I'm solving now is: currently OWNERS files
>> contain many long-time inactive, non-OOO owners. The goal is to remove
>> them. Do you agree with the goal?
>>
>> The "long-time inactive" part is covered by looking into their review /
>> commit activities in the past 6 months.
>>
>> The "non-OOO" part is tricky because currently there is no way to get the
>> information from the tool. I'm not sure if giving an 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Peter Boström]

I agree with the policy to not remove owners because they are on leave.
Owners plus OOO status should inform owner selection and we still need that
because we shouldn't remove owners while they are out for a week but they
sure shouldn't be used during that week.

If OOO is an efficient signal this should be sufficient without force
removing folks on leave. If OOO isn't a sufficient signal and introduces
review latency we should look at why that is instead and fix our tooling.

If I may spitball another reason for latency we should maybe also surface
how long folks review queues are and perhaps use that to help guide owners
selection.

Also because I'm already up on my soapbox, if you're fast and not
overloaded and want to help with the review load, please put something like
"fast" or "more reviews plz" in your Gerrit status to encourage more
reviews your way (me inspired by ellyjones@ here). Encourage others to do
the same and reward this as a community contribution.

On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:

> 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 <
> blink-dev@chromium.org> wrote:
>
>>
>>
>> On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:
>>
>>>
>>>
>>> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
>>> 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
 ,
 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
>>> 
>>>  (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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Peter Boström]

I specifically disagree with the "for this cycle, I'm planning to remove
the inactive owners unconditionally" bit. If you suggest this removal but
you defer to local OWNERS to make the call I think that's fine to send out
removal CLs for. I would encourage you and/or the reviewers to look at the
removed owner's calendar if they're internal and confirm that they've not
been on long-term leave.

I don't think that 85/6850 OWNERS entries are a large enough problem to
bypass local owner approval for this (i.e. land it unconditionally). If
some of them are owners in very active folders and have had a lot of
reviews assigned to them, sure. This is about 1% of ownership entries if
I'm understanding this correctly. How badly are they contributing to review
latency in order to override "and they are not on a leave during that time"
as a policy? I like this policy, and it's supposed to still be the policy.

If any of these OWNERS entries had a significant amount of reviews assigned
to them during this period (i.e. sorta-evidence of latency), you could use
that as additional justification in the CLs to get the OWNERS to approve
the (possibly temporary) removal.

For fixing review latency which is the real goal presumably, I think
handling OOO and load balancing are both more important vectors here. We
shouldn't need to get inactive owners down to 0 to solve review latency.

On Thu, Jul 28, 2022 at 6:47 PM Kentaro Hara  wrote:

> +1 to improving the tool to handle OOO owners.
>
> Let's step back. The problem I'm solving now is: currently OWNERS files
> contain many long-time inactive, non-OOO owners. The goal is to remove
> them. Do you agree with the goal?
>
> The "long-time inactive" part is covered by looking into their review /
> commit activities in the past 6 months.
>
> The "non-OOO" part is tricky because currently there is no way to get the
> information from the tool. I'm not sure if giving an option to manually
> flip the decision is a great solution because if they are long-time OOO,
> they are not likely to be looking at this email and get removed anyway.
> Then I thought that it's more reasonable to execute the clean-up process
> unconditionally while explicitly saying that they can explain the situation
> and re-add themselves when they are back (where they can refer to this
> email thread).
>
> What do you think?
>
>
>
> On Fri, Jul 29, 2022 at 1:41 AM K. Moon  wrote:
>
>> On a related topic, one challenge that I have with OOO status is that
>> it's hard to update all the places where you might want to note that (each
>> Gerrit repository, Slack, external Gmail, internal Gmail, Calendar, ...).
>> For Googlers, I have decent tooling for checking whether they're OOO, in my
>> time zone, or whatever (although better integration would save me time),
>> but if I wasn't a Googler (and didn't have access to those tools), or I
>> want to check the status of a non-Googler, it's often not as easy, or
>> impossible.
>>
>> It'd be great if there were One Tool that (opt-in) tracked OOO status in
>> a centralized directory, and worked whether or not you had a Google or
>> Chromium account.
>>
>> For parts of the tree that need to balance between a large number of
>> reviewers, something like gwsq probably would be a decent solution.
>> (//ipc/SECURITY_OWNERS is using this now; I think trees like //content
>> would benefit as well.)
>>
>> On Thu, Jul 28, 2022 at 8:56 AM Peter Boström  wrote:
>>
>>> I agree with the policy to not remove owners because they are on leave.
>>> Owners plus OOO status should inform owner selection and we still need that
>>> because we shouldn't remove owners while they are out for a week but they
>>> sure shouldn't be used during that week.
>>>
>>> If OOO is an efficient signal this should be sufficient without force
>>> removing folks on leave. If OOO isn't a sufficient signal and introduces
>>> review latency we should look at why that is instead and fix our tooling.
>>>
>>> If I may spitball another reason for latency we should maybe also
>>> surface how long folks review queues are and perhaps use that to help guide
>>> owners selection.
>>>
>>> Also because I'm already up on my soapbox, if you're fast and not
>>> overloaded and want to help with the review load, please put something like
>>> "fast" or "more reviews plz" in your Gerrit status to encourage more
>>> reviews your way (me inspired by ellyjones@ here). Encourage others to
>>> do the same and reward this as a community contribution.
>>>
>>> On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:
>>>
 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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Peter Boström]

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  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  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
>> 
>> 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
>> 
>> and be readded through the standard OWNERS nomination process.
>>
>> Specifically, people listed in this spreadsheet
>> 
>> 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 blink-dev+unsubscr...@chromium.org.
>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com
>> 
>> .
>>
> --
> --
> Chromium Developers mailing list: chromium-...@chromium.org
> 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 chromium-dev+unsubscr...@chromium.org.
> 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
> 
> .
>

-- 
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 blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGFX3sFB9G8R2MyHT6rjVtEFRAKMeyCTH6Yu0DYqUOfLPCxCBw%40mail.gmail.com.

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Adam Rice]

+1 to removing inactive OWNERS. It should not require insider knowledge to
find someone to review your CL.

While it would be great if our tools were better at handling OOO OWNERS,
the current state of affairs is that they aren't. It is easy enough for
someone to re-add themselves to OWNERS when coming back from a long leave,
and I don't think it is unreasonably burdensome.

On Fri, 29 Jul 2022 at 10:47, Kentaro Hara  wrote:

> +1 to improving the tool to handle OOO owners.
>
> Let's step back. The problem I'm solving now is: currently OWNERS files
> contain many long-time inactive, non-OOO owners. The goal is to remove
> them. Do you agree with the goal?
>
> The "long-time inactive" part is covered by looking into their review /
> commit activities in the past 6 months.
>
> The "non-OOO" part is tricky because currently there is no way to get the
> information from the tool. I'm not sure if giving an option to manually
> flip the decision is a great solution because if they are long-time OOO,
> they are not likely to be looking at this email and get removed anyway.
> Then I thought that it's more reasonable to execute the clean-up process
> unconditionally while explicitly saying that they can explain the situation
> and re-add themselves when they are back (where they can refer to this
> email thread).
>
> What do you think?
>
>
>
> On Fri, Jul 29, 2022 at 1:41 AM K. Moon  wrote:
>
>> On a related topic, one challenge that I have with OOO status is that
>> it's hard to update all the places where you might want to note that (each
>> Gerrit repository, Slack, external Gmail, internal Gmail, Calendar, ...).
>> For Googlers, I have decent tooling for checking whether they're OOO, in my
>> time zone, or whatever (although better integration would save me time),
>> but if I wasn't a Googler (and didn't have access to those tools), or I
>> want to check the status of a non-Googler, it's often not as easy, or
>> impossible.
>>
>> It'd be great if there were One Tool that (opt-in) tracked OOO status in
>> a centralized directory, and worked whether or not you had a Google or
>> Chromium account.
>>
>> For parts of the tree that need to balance between a large number of
>> reviewers, something like gwsq probably would be a decent solution.
>> (//ipc/SECURITY_OWNERS is using this now; I think trees like //content
>> would benefit as well.)
>>
>> On Thu, Jul 28, 2022 at 8:56 AM Peter Boström  wrote:
>>
>>> I agree with the policy to not remove owners because they are on leave.
>>> Owners plus OOO status should inform owner selection and we still need that
>>> because we shouldn't remove owners while they are out for a week but they
>>> sure shouldn't be used during that week.
>>>
>>> If OOO is an efficient signal this should be sufficient without force
>>> removing folks on leave. If OOO isn't a sufficient signal and introduces
>>> review latency we should look at why that is instead and fix our tooling.
>>>
>>> If I may spitball another reason for latency we should maybe also
>>> surface how long folks review queues are and perhaps use that to help guide
>>> owners selection.
>>>
>>> Also because I'm already up on my soapbox, if you're fast and not
>>> overloaded and want to help with the review load, please put something like
>>> "fast" or "more reviews plz" in your Gerrit status to encourage more
>>> reviews your way (me inspired by ellyjones@ here). Encourage others to
>>> do the same and reward this as a community contribution.
>>>
>>> On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:
>>>
 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 <
 blink-dev@chromium.org> wrote:

>
>
> On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:
>
>>
>>
>> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
>> 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, 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Manuel Rego Casasnovas]

On 28/07/2022 16:33, 'Matt Menke' via Chromium-dev wrote:
> 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.

Just for completeness, there are some OWNERS files that require a
nomination process. At least third_party/blink/renderer/core/OWNERS
requires that:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/OWNERS

Cheers,
  Rego

-- 
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 blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/7f52c766-8692-b906-78c5-20d263f23b1f%40igalia.com.

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Glen Robertson]

+1 to needing better tools for handling OOO, and big +1 to using gwsq when
there are many reviewers to balance.

Given that long leave is potentially 6 months, but unlikely to be 12 months
(citation needed), how about changing the criteria to 12 months? Then it
sort of covers both "long-tme inactive" and "non-OOO". Regardless, until we
have better tooling for OOO, I think it's reasonable to remove people who
are currently inactive and they can be re-added later.

On Fri, 29 Jul 2022 at 11:47, Kentaro Hara  wrote:

> +1 to improving the tool to handle OOO owners.
>
> Let's step back. The problem I'm solving now is: currently OWNERS files
> contain many long-time inactive, non-OOO owners. The goal is to remove
> them. Do you agree with the goal?
>
> The "long-time inactive" part is covered by looking into their review /
> commit activities in the past 6 months.
>
> The "non-OOO" part is tricky because currently there is no way to get the
> information from the tool. I'm not sure if giving an option to manually
> flip the decision is a great solution because if they are long-time OOO,
> they are not likely to be looking at this email and get removed anyway.
> Then I thought that it's more reasonable to execute the clean-up process
> unconditionally while explicitly saying that they can explain the situation
> and re-add themselves when they are back (where they can refer to this
> email thread).
>
> What do you think?
>
>
>
> On Fri, Jul 29, 2022 at 1:41 AM K. Moon  wrote:
>
>> On a related topic, one challenge that I have with OOO status is that
>> it's hard to update all the places where you might want to note that (each
>> Gerrit repository, Slack, external Gmail, internal Gmail, Calendar, ...).
>> For Googlers, I have decent tooling for checking whether they're OOO, in my
>> time zone, or whatever (although better integration would save me time),
>> but if I wasn't a Googler (and didn't have access to those tools), or I
>> want to check the status of a non-Googler, it's often not as easy, or
>> impossible.
>>
>> It'd be great if there were One Tool that (opt-in) tracked OOO status in
>> a centralized directory, and worked whether or not you had a Google or
>> Chromium account.
>>
>> For parts of the tree that need to balance between a large number of
>> reviewers, something like gwsq probably would be a decent solution.
>> (//ipc/SECURITY_OWNERS is using this now; I think trees like //content
>> would benefit as well.)
>>
>> On Thu, Jul 28, 2022 at 8:56 AM Peter Boström  wrote:
>>
>>> I agree with the policy to not remove owners because they are on leave.
>>> Owners plus OOO status should inform owner selection and we still need that
>>> because we shouldn't remove owners while they are out for a week but they
>>> sure shouldn't be used during that week.
>>>
>>> If OOO is an efficient signal this should be sufficient without force
>>> removing folks on leave. If OOO isn't a sufficient signal and introduces
>>> review latency we should look at why that is instead and fix our tooling.
>>>
>>> If I may spitball another reason for latency we should maybe also
>>> surface how long folks review queues are and perhaps use that to help guide
>>> owners selection.
>>>
>>> Also because I'm already up on my soapbox, if you're fast and not
>>> overloaded and want to help with the review load, please put something like
>>> "fast" or "more reviews plz" in your Gerrit status to encourage more
>>> reviews your way (me inspired by ellyjones@ here). Encourage others to
>>> do the same and reward this as a community contribution.
>>>
>>> On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:
>>>
 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 <
 blink-dev@chromium.org> wrote:

>
>
> On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:
>
>>
>>
>> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
>> 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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Kentaro Hara]

+1 to improving the tool to handle OOO owners.

Let's step back. The problem I'm solving now is: currently OWNERS files
contain many long-time inactive, non-OOO owners. The goal is to remove
them. Do you agree with the goal?

The "long-time inactive" part is covered by looking into their review /
commit activities in the past 6 months.

The "non-OOO" part is tricky because currently there is no way to get the
information from the tool. I'm not sure if giving an option to manually
flip the decision is a great solution because if they are long-time OOO,
they are not likely to be looking at this email and get removed anyway.
Then I thought that it's more reasonable to execute the clean-up process
unconditionally while explicitly saying that they can explain the situation
and re-add themselves when they are back (where they can refer to this
email thread).

What do you think?



On Fri, Jul 29, 2022 at 1:41 AM K. Moon  wrote:

> On a related topic, one challenge that I have with OOO status is that it's
> hard to update all the places where you might want to note that (each
> Gerrit repository, Slack, external Gmail, internal Gmail, Calendar, ...).
> For Googlers, I have decent tooling for checking whether they're OOO, in my
> time zone, or whatever (although better integration would save me time),
> but if I wasn't a Googler (and didn't have access to those tools), or I
> want to check the status of a non-Googler, it's often not as easy, or
> impossible.
>
> It'd be great if there were One Tool that (opt-in) tracked OOO status in a
> centralized directory, and worked whether or not you had a Google or
> Chromium account.
>
> For parts of the tree that need to balance between a large number of
> reviewers, something like gwsq probably would be a decent solution.
> (//ipc/SECURITY_OWNERS is using this now; I think trees like //content
> would benefit as well.)
>
> On Thu, Jul 28, 2022 at 8:56 AM Peter Boström  wrote:
>
>> I agree with the policy to not remove owners because they are on leave.
>> Owners plus OOO status should inform owner selection and we still need that
>> because we shouldn't remove owners while they are out for a week but they
>> sure shouldn't be used during that week.
>>
>> If OOO is an efficient signal this should be sufficient without force
>> removing folks on leave. If OOO isn't a sufficient signal and introduces
>> review latency we should look at why that is instead and fix our tooling.
>>
>> If I may spitball another reason for latency we should maybe also surface
>> how long folks review queues are and perhaps use that to help guide owners
>> selection.
>>
>> Also because I'm already up on my soapbox, if you're fast and not
>> overloaded and want to help with the review load, please put something like
>> "fast" or "more reviews plz" in your Gerrit status to encourage more
>> reviews your way (me inspired by ellyjones@ here). Encourage others to
>> do the same and reward this as a community contribution.
>>
>> On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:
>>
>>> 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 <
>>> blink-dev@chromium.org> wrote:
>>>


 On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:

>
>
> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
> 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).

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[K. Moon]

On a related topic, one challenge that I have with OOO status is that it's
hard to update all the places where you might want to note that (each
Gerrit repository, Slack, external Gmail, internal Gmail, Calendar, ...).
For Googlers, I have decent tooling for checking whether they're OOO, in my
time zone, or whatever (although better integration would save me time),
but if I wasn't a Googler (and didn't have access to those tools), or I
want to check the status of a non-Googler, it's often not as easy, or
impossible.

It'd be great if there were One Tool that (opt-in) tracked OOO status in a
centralized directory, and worked whether or not you had a Google or
Chromium account.

For parts of the tree that need to balance between a large number of
reviewers, something like gwsq probably would be a decent solution.
(//ipc/SECURITY_OWNERS is using this now; I think trees like //content
would benefit as well.)

On Thu, Jul 28, 2022 at 8:56 AM Peter Boström  wrote:

> I agree with the policy to not remove owners because they are on leave.
> Owners plus OOO status should inform owner selection and we still need that
> because we shouldn't remove owners while they are out for a week but they
> sure shouldn't be used during that week.
>
> If OOO is an efficient signal this should be sufficient without force
> removing folks on leave. If OOO isn't a sufficient signal and introduces
> review latency we should look at why that is instead and fix our tooling.
>
> If I may spitball another reason for latency we should maybe also surface
> how long folks review queues are and perhaps use that to help guide owners
> selection.
>
> Also because I'm already up on my soapbox, if you're fast and not
> overloaded and want to help with the review load, please put something like
> "fast" or "more reviews plz" in your Gerrit status to encourage more
> reviews your way (me inspired by ellyjones@ here). Encourage others to do
> the same and reward this as a community contribution.
>
> On Thu, Jul 28, 2022 at 8:15 AM K. Moon  wrote:
>
>> 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 <
>> blink-dev@chromium.org> wrote:
>>
>>>
>>>
>>> On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:
>>>


 On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
 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
> ,
> 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
 
  (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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[K. Moon]

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 <
blink-dev@chromium.org> wrote:

>
>
> On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:
>
>>
>>
>> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara 
>> 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
>>> ,
>>> 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
>> 
>>  (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
 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

['Matt Menke' via blink-dev]

On Thu, Jul 28, 2022 at 10:29 AM Ali Juma  wrote:

>
>
> On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara  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
>> ,
>> 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
> 
>  (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 
>> wrote:
>>
>>> > Having your name on 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Ali Juma]

On Thu, Jul 28, 2022 at 6:07 AM Kentaro Hara  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
> ,
> 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

(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.


>
> 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 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 
> 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
>> 
>> 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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Kentaro Hara]

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
,
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?

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 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  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
> 
> 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
>  (as
> list, Google internal only
> )
> many accounts that no longer exist (or 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Glen Robertson]

> 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

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
 (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?

On Thu, 28 Jul 2022 at 07:45, Pavel Feldman  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 k...@chromium.org 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 
>>> 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  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 
> 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
>> 
>> 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 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Pavel Feldman]

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 k...@chromium.org 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  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  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  
 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 
> 
>  
> 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 
> 
>  
> and be readded through the standard OWNERS nomination process.
>
> Specifically, people listed in this spreadsheet 
> 
>  
> 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 blink-dev+...@chromium.org.


> To view this discussion on the web visit 
> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com
>  
> 
> .
>
 -- 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

['Matt Menke' via blink-dev]

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 k...@chromium.org 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  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  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  
>>> 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 
 
  
 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 
 
  
 and be readded through the standard OWNERS nomination process.

 Specifically, people listed in this spreadsheet 
 
  
 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 blink-dev+...@chromium.org.
 To view this discussion on the web visit 
 https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com
  
 
 .

>>> -- 
>>> -- 
>>> Chromium Developers mailing list: chromi...@chromium.org
>>> 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 chromium-dev...@chromium.org.
>>> To view this discussion on the web visit 
>>> 

Re: [chromium-dev] Re: [blink-dev] Inactive OWNERS cleanup

[Ken Russell]

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  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  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 
>> 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
>>> 
>>> 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
>>> 
>>> and be readded through the standard OWNERS nomination process.
>>>
>>> Specifically, people listed in this spreadsheet
>>> 
>>> 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 blink-dev+unsubscr...@chromium.org.
>>> To view this discussion on the web visit
>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jyArLjDp0ixPu%2BCSZ9NVrn0M1GwNFiJqiPGRE1f0mrbfQ%40mail.gmail.com
>>> 
>>> .
>>>
>> --
>> --
>> Chromium Developers mailing list: chromium-...@chromium.org
>> 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 chromium-dev+unsubscr...@chromium.org.
>> 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
>> 
>> .
>>
> --
> --
> Chromium Developers mailing list: chromium-...@chromium.org
> 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 chromium-dev+unsubscr...@chromium.org.
> To view this