[owners] Is it possible to declare different owners for different lines in a file? about amp-github-apps HOT 6 CLOSED

I wonder if that's getting a bit deep in the weeds for the OWNERS bot? That also means the OWNERS file has to be updated any time a dependency is added, since it'll change the line numbers (adding the weird case of an OWNERS file allowing modification of itself somehow?). Do we have other examples where this would be useful? Otherwise it might be cleaner to just have a standalone renovate-reviewer-bot who monitors renovate CLs, identifies the author that added the dependency, and brings them in. I supposed we could have the OWNERS file specify who owns the dependencies, but that still gets us into territory where adding dependencies means updating owners files which seems like an ugly cycle.

from amp-github-apps.

@rcebulko Good questions.

I wonder if that's getting a bit deep in the weeds for the OWNERS bot? That also means the OWNERS file has to be updated any time a dependency is added, since it'll change the line numbers (adding the weird case of an OWNERS file allowing modification of itself somehow?).

A less brittle way than hard-coded line numbers is to choose reviewers based on the name of the dependency being upgraded and not line numbers.

Do we have other examples where this would be useful? Otherwise it might be cleaner to just have a standalone renovate-reviewer-bot who monitors renovate CLs, identifies the author that added the dependency, and brings them in.

I posted #284 (comment) to explain why I think the owners bot should be the one to assign reviewers for renovate PRs.

I supposed we could have the OWNERS file specify who owns the dependencies, but that still gets us into territory where adding dependencies means updating owners files which seems like an ugly cycle.

In practice, we've only needed a small percentage of renovate PRs to be reviewed by whoever originally added (or currently maintains) the code that uses the dependency.

For dependencies that don't have an owner, you can assign the PR to the build-cop, and post a comment asking them to merge if it looks straightforward (and passes Travis), and re-assign if it contains non-trivial or breaking changes. With this model, the owners bot will not have to be re-deployed, since it can get the latest owner info from the OWNERS.yaml file in the project root.

WDYT? (Might be worth writing up a one-pager doc that we review separately. Also, this is P3 😃)

from amp-github-apps.

I think a one-pager (at least, once this P3 gets closer to the front of the line) would be a good idea; going to collect a bit more info and will aggregate in write-up. I see a lot of different angles we could take (choosing based on who added the dependency, explicitly specify dependency owners/users) and all have a couple pros and cons. Re: your comment on #284, I definitely see how renovate PRs have an ownership aspect to them. I could also see wanting to consider the level of the change (patch/minor/major) when determining who can sign off on the upgrade ie. a major upgrade that one extension thinks is great might break another.

Is anyone be allowed to add dependencies, generally?

from amp-github-apps.

Is anyone be allowed to add dependencies, generally?

Generally, yes¹.

[1] There are two types: dependencies (code is shipped with the runtime) and devDependencies (used for tooling, not shipped). We only upgrade the latter via renovate. Most upgrades are straightforward and don't require a careful review if all tests pass.

from amp-github-apps.

@rsimha Ahh that makes sense + a big difference

from amp-github-apps.

Closing this as a duplicate of #284

from amp-github-apps.