Introducing code owners | Hacker News Reader

Introducing code owners

github.com

121 pointstomschlick14 comments

This is really excellent. Making sure people get put onto the right PRs is a hassle, and anything that makes code review easier is a huge plus in my book.

This post explains the use of OWNERS by the Chromium project, which inspired this GitHub feature: https://meowni.ca/posts/chromium-owners/

It's not just by the Chromium project, but this is a standard used by Google in general in their code repo.

I was sort of hoping this would determine code ownership based on who wrote the lines being modified in the patch. This solution is simple and still useful, though.

GitHub actually already sort-of has that with their "Suggested reviewers":

https://help.github.com/articles/requesting-a-pull-request-r...

> You can request a review from either a suggested or specific person. Suggested reviewers are based on git blame data.

It just needs a trick to auto-add suggested reviewers without manual process.

> # Order is important. The last matching pattern has the most precedence.

That seems like a misfeature. Whenever a format does this (XSLT? Some Apache config?), i end up confused at some point.

I get that in this case, it lets you write your backstop rules at the top, which is nice. But it will trip people up.

In fact, i'd say any kind of order dependence is a misfeature. CSS-like precedence rules based on the specificity of the match are good. I have no idea how you'd define those for regular expressions, though. Perhaps they should just allow explicit weights on the rules?

By the way, if anyone is wondering what colour to paint their bikeshed, just give me a shout.

I think the reason for this is to match how gitattributes[1] works (which is how gitignore works). It's not /necessarily/ a misfeature because it matches the existing Git interface semantics, though I do agree it might be confusing to people who are not familiar with gitattributes (you could think of CODEOWNERS as a kind-of extension of gitattributes).

[1]: https://git-scm.com/docs/gitattributes

> Perhaps they should just allow explicit weights on the rules?

Wouldn't that be equivalent to order-dependent format, but with more noise?

As it has been said, it follows other git features. I disagree that rules based on specificity are better. Sometimes I had a hard time reasoning about conflicting CSS rules with different levels of specificity, while I never had any problem with precedence based rules.

In any case, every feature like this should have an easy way to preview what a change does.

> any kind of order dependence is a misfeature

I think you're referring primarily to config in this case but it seems pretty standard to me, programming in general is a bit of an order dependence dance, and it seems more natural (to me) that an event that occurs later in time (matching last) should overwrite any earlier instance.

Do the code owners still need to approve? Or can I just add random owners to the file and have them get notifications without them wanting to?

Users and teams need to have write access to a repo to be considered code owners. This prevents it from being used to harass random users.

we already had this for some time in our gerrit review system

Reply on news.ycombinator.com