Re: Specifying code review notifications

That SGTM as well. James, is your open PR just blocked on someone reviewing
it, or are you awaiting objections?

On Mon, Jul 10, 2017 at 6:53 PM Reilly Grant <reillyg@google.com> wrote:

> I'm in favor of option 2 as it allows us to use the GitHub approved system
> while both having an automated system for keeping the format of the file
> correct and keeping the owner information in what I believe is a more
> readable format.
>
> On Mon, Jul 10, 2017, 07:26 James Graham <james@hoppipolla.co.uk> wrote:
>
>> GitHub recently added support for specifying code ownership [1]. This is
>> roughly equivalent to the system we already have with OWNERS files in
>> various directories, and which we use to get people notified on PRs they
>> care about. However it differs in a couple of ways (some positive and
>> some negative):
>>
>> * Some additional support in GitHub. We mostly don't care about this
>> because we don't want to require that reviewers are listed in the
>> *OWNERS file(s), but you get some additional icons (arguably misleading
>> for our use case, since we aren't specifying any enforced ownership),
>> and there might be more support in the future.
>>
>> * Add entries go in a single file with precedence rules rather than one
>> file per directory with a flat list of owners.
>>
>> There are also some implementation differences:
>>
>> * The GitHub system puts most implementation complexity in GH rather
>> than in services that we (or, rather, tobie) run.
>>
>> At this point we have several options:
>>
>> 1) Go all-in on the new feature. Make people edit the CODEOWNERS file to
>> get notification for PRs. This has the advantage that we are doing the
>> "GitHub approved thing". It has the disadvantage that the format of the
>> file is IMO rather complex for our case where there are hundreds of
>> directories all with different owners. I think we would have to invest
>> in tooling to ensure that people didn't unintentionally edit the file in
>> a way that clobbers other people's notification requests.
>>
>> 2) Use the new system, but with the input being OWNERS file from the new
>> system, and a `wpt owners-update` tool that must be run to generate the
>> CODEOWNERS file. This would avoid the problems with the new format, but
>> would have the disadvantage of having to run a tool for each update.
>> Travis could check the tool was run, but it would still require manual
>> work.
>>
>> 3) Use the existing system. This would mean no changes, but also mean
>> giving up the option to turn off the (parts of the) external bot we are
>> using to recreate this feature.
>>
>> Opinions?
>>
>> I have an implementation of 2) at [2], which also includes a generated
>> CODEOWNERS file [3] so you can get an idea of the complexity. I think
>> that creating good tools to help with 1) would be harder,
>> implementation-wise, but I don't know quite how hard.
>>
>>
>> [1] https://github.com/blog/2392-introducing-code-owners
>> [2] https://github.com/w3c/web-platform-tests/pull/6506
>> [3]
>>
>> https://github.com/w3c/web-platform-tests/blob/def1efdedfab188a4927cdd7e516491c01bd02ff/CODEOWNERS
>>
>>