CODEOWNERS: Split codeowners for the documentation

[pchaigno]

With recent changes to the review process, @cilium/docs was renamed to @cilium/docs-structure to clarify that reviews from that team should focus on the documentation's structure rather than its technical content.

Of course, we still need reviews for the technical content. So the next step, implemented in this pull request, is to assign each of the different reviewer team their own pages in the documentation.

[joestringer]

Thanks for picking this up, I was just starting to think about this :-)

I don't think there's any good way to test this unfortunately, but I noticed a couple of things that differ from the existing configuration:

• Current configuration specifies /Documentation/ first as a default for all files under that directory, then overrides the owner with subsequent configuration
• Unless another path above overlaps with the path, there should be no need to add another entry specifically for a given path.

Also, it would be nice to sort alphabetically, with shorter paths first just to make it easier to read & find individual paths if possible (modulo the ordering rules which I believe are that the last entry that matches will define the owners for the given path)

[joestringer]

Isn't this already applied by matching /Documentation/ for this group? (likewise for similar other paths which only have the @cilium/docs-structure group as owner, unless there may be a conflict with another line in the file)

[pchaigno]

Yep, that's a good question. I wanted to keep (1) the entries I've already classified as being @cilium/docs-structure's responsibility separate from (2) entries I haven't classified under any codeowner because it's unclear (e.g., Documentation/gettingstarted/local-redirect-policy.rst).

I'm not sure that makes sense, especially considering it makes the whole thing more complex. But it's clearly easier to change the PR from separated entries to single @cilium/docs-structure entry than the other way around 😄

[pchaigno]

Current configuration specifies /Documentation/ first as a default for all files under that directory, then overrides the owner with subsequent configuration

Ouch, yes. I messed up the order. Will fix.

[pchaigno]

/Documentation/ is actually the only entry in CODEOWNERS that conflicts with (& is overridden by) other /Documentation/ entries. I don't expect that to change because, when we have a subdir in Documentation/, it usually doesn't belong to any specific codeowner or it belongs entirely to a single codeowner. Given that, I just sorted the entries alphabetically, without trying to put shorter paths first. I hope it's better; if not, let's try something else.

(Ideally, I think we would generate our CODEOWNERS from a different, user-friendlier syntax. But there's still a bit of work needed to get there.)

[joestringer]

There's still some entries that look like they might be redundant, but everything that's there looks right to me 👍

[jibi]

LGTM 📖

[qmonnet]

I understand the /Documentation/ @cilium/docs-structure will be a catch-all for any documentation page which does not a more specific entry, but do we have something in place to checking that we didn't forget some entries, or that we won't forget to update the CODEOWNERS when adding new pages?

[pchaigno]

I understand the /Documentation/ @cilium/docs-structure will be a catch-all for any documentation page which does not a more specific entry, but do we have something in place to checking that we didn't forget some entries, or that we won't forget to update the CODEOWNERS when adding new pages?

For bpf/, I think we have checkpatch. For other entries, we don't have an equivalent. This would be good to have.

One other idea I had was to assign the catch-all entry to both @cilium/docs-structure and @cilium/janitors. Then janitors would check why their review was requested and re-assign documentation pages to the appropriate teams (whether @cilium/docs-structure alone or some other team). That way we would slowly reduce the catch-all.

[qmonnet]

One other idea [...]

Yes, adding janitors to the catch-all sounds like a good idea to me!

[aanm]

What will happen to this nonexistantteam?

[pchaigno]

It will continue to not exist 😄

See ba5f196 for context.