[owners] implement file level only owner about amp-github-apps HOT 9 CLOSED

Note: Pattern-based rules are NOT planned to support subdirectories/full paths at this time. Unless a need arises, the appropriate subdirectory can have its own OWNERS file.

from amp-github-apps.

migrated from: google/github-owners-bot#4

from amp-github-apps.

Syntax proposal

Patterns

Support extremely simple pattern-based paths

• * represents any string of characters (glob, not regex)
• by default, applies recursively (ie. *.css applies to all CSS files in folder and subdirectories)
• with ./ prefix, applies only to current directory (ie. ./*.css applies to ./main.css but not ./foo/bar.css)
• if no glob pattern is present, it's an exact match (ie. ./package.json matches only package.json in that directory, while package.json matches all package.json files in all subdirectories as well)
• multiple comma-separated patterns may be specified (ex. *.js, *.css)

Pattern ownership is additive

• if user1 is listed in the OWNERS file as owning the directory, then user1 also owns any pattern-based rules

For the purposes of the reviewer selection algorithm, pattern-based rules have the same depth as the directory ownership rules.

Rules

Pattern-based rules will be defined as a dict in the OWNERS.yaml file, adjacent in the list to the file owners. They key will be the pattern, while the value will be the list of owners of files affected by the pattern. Example:

- alice
- bob
- ./package.json:
  - charlie

In the case of an OWNERS file with pattern rules but no directory owners, the dict should still be within a list. Example:

- *.css:
  - alice
  - bob

from amp-github-apps.

@rsimha @erwinmombay Any opinions?

from amp-github-apps.

could we follow glob matching rules were **/* is needed for recursive matching? just to align with glob rules properly

from amp-github-apps.

@erwinmombay So instead of *.js to match recursively and ./*.js to match in same-directory only, use *.js for same-directory and **/*.js for recursive?

from amp-github-apps.

@danielrozenberg mentioned looking into various glob syntax parsers for handling these rules. That would be useful in that it would be fully-featured, but my concern is that it would enable more complexity than should be necessary. .gitignore and similar configs use globs because they're a convenient way to represent at times fairly complex sets of conditions, and usually from a single top-level file across tons of nested subdirectories.

I feel like we may not want to support that level of complexity in OWNERS files, especially since subfolders can easily have their own OWNERS file, keeping rules anchored to the directories they apply to. Additionally, the OWNERS hierarchy gives a clear precedence structure for rules, which can get more confusing the more details we add (ie. negation in particular).

TL;DR - It would be possible to support more complex glob patterns, but I don't feel like those complex patterns would add anything useful in practice.

@rsimha @erwinmombay Are there any examples you can think of (that already exist, not in a potential future) where full glob match would be more useful than simple wildcard patterns?

from amp-github-apps.

@rcebulko agree with your points to not give so much flexibility in the rules matching. the only thing i could think of is terseness of the config but id rather be verbose in the configuration so i do agree that simple wildcard patterns would be the way to go

from amp-github-apps.

Closed by #403

from amp-github-apps.