Re: Specifying code review notifications

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