ci: add just check-codeowners - #587
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
4727915 to
6ffcc2e
Compare
tonyandrewmeyer
left a comment
tonyandrewmeyer
left a comment
There was a problem hiding this comment.
Looks good, and the currently failing cases provide a nice example of what it will look like if you get it wrong.
| """Return a list of problems found between the CODEOWNERS `entries` and the tracked tree. | ||
|
|
||
| Everything is checked against the tracked tree (`children`) rather than the filesystem, so | ||
| untracked and ignored paths are irrelevant. A path is a tracked directory iff it's a key in |
There was a problem hiding this comment.
I'm always a bit iffy (heh) about using "iff". This is an internal script so it probably doesn't matter, but I wonder how widely people understand it versus thinking someone has typo'd "if".
| problems.append(f'CODEOWNERS entry points at a missing path: {pattern}') | ||
| elif (target in children) != pattern.endswith('/'): | ||
| problems.append( | ||
| f'CODEOWNERS entry must have a trailing slash iff it is a dir: {pattern}' |
There was a problem hiding this comment.
Ah, though this is public. I think the one above could be left in place but we could spell it out here.
There was a problem hiding this comment.
I'll split this into two types of problems: directory with no /, and non-directory with /.
| def map_children(files: list[pathlib.Path]) -> dict[pathlib.Path, list[pathlib.Path]]: | ||
| """Map each tracked directory to its sorted immediate children (files and subdirectories). | ||
|
|
||
| The repository root is keyed as `pathlib.Path()` (i.e. `.`). A path is a directory iff it |
There was a problem hiding this comment.
Are we avoiding Latin in this repo?
There was a problem hiding this comment.
Apparently not very well -- some ag searching shows:
{
"e.g.": 102,
"i.e.": 16,
"etc.": 13,
"verbatim": 5,
"vice versa": 1,
"ergo": 1
}But on the bright side, no hits for:
[
"N.B.",
"a priori",
"ad hoc",
"ad hominem",
"bona fide",
"cf.",
"de facto",
"de jure",
"et al.",
"et cetera",
"et seq.",
"ibid.",
"in situ",
"in vitro",
"inter alia",
"mutatis mutandis",
"per se",
"quid pro quo",
"sic",
"sine qua non",
"status quo",
"viz.",
"vs."
]| if problems: | ||
| print('\n'.join(problems)) | ||
| else: | ||
| print('No problems found with CODEOWNERS :)') |
There was a problem hiding this comment.
I like the old-school smiley to distinguish from the AI favoured emoji ✅
3 participants
This PR adds
just check-codeowners, which ensures that:interfaces/).CI currently fails due to four unowned interfaces. Blocked by landing PRs to add codeowners for these (TODO):
ingressandingress_per_unitshould presumably be@canonical/observabilityspark_service_accountandzookeeperseem to be@canonical/data-bigdataResolves #548.