Feat: codeowners orphans #58
Conversation
| "ice-cream-flavor": "vanilla", | ||
| "owner": "@jdunck", | ||
| "timezone": "PST", | ||
| }, |
| const entrySatisfaction: [OwnerEntry, boolean][] = this.ownerEntries.map((entry) => { | ||
| const full = path.join(this.codeownersDirectory, entry.path); | ||
| try { | ||
| const iterator = globIterateSync(full, {}); |
There was a problem hiding this comment.
This checks if the glob has any entries without walking the whole result, just terminates the Generator as soon as the answer is known. (TBH I am not sure if glob is lazy or eagerly precomputes the result and just supports this interface.)
There was a problem hiding this comment.
I would have suggested using the newer built-in glob feature of NodeJS v22 instead of a new dependency, but it has no comparable option for lazy evaluation.
I double checked the implementation in the package's source and I think it is indeed lazily evaluated, so this is an ideal approach.
noahm
left a comment
There was a problem hiding this comment.
This is a smart addition! I'm going to be doing a bit of additional testing with it and hopefully get it into a release this weekend!
Thanks for the contribution!
| const entrySatisfaction: [OwnerEntry, boolean][] = this.ownerEntries.map((entry) => { | ||
| const full = path.join(this.codeownersDirectory, entry.path); | ||
| try { | ||
| const iterator = globIterateSync(full, {}); |
There was a problem hiding this comment.
I would have suggested using the newer built-in glob feature of NodeJS v22 instead of a new dependency, but it has no comparable option for lazy evaluation.
I double checked the implementation in the package's source and I think it is indeed lazily evaluated, so this is an ideal approach.
|
@jdunck This is now published as |
Very cool, thanks - just a bit of usage outside the original PR/tests -- I have a local project with 354k files, and a codeowner file of 84 rules -- it correctly lists orphans in that file. $ time node ~/code/open/codeowners/dist/cli.js orphans
...
node ~/code/open/codeowners/dist/cli.js orphans 1.12s user 2.16s system 113% cpu 2.898 totalThis is on a Macbook M4 Pro -- I haven't tested on linux or windows, though. |
|
I realized I didn't update the README with the command, want me to do that, or leave it to you? |
|
And one more note -- I had a need for some other things around codeowners files, so I started these two things: The idea of the -corpus repo is to build a bunch of commits with various edge cases. It's not super-mature, but maybe useful to you. And the -tools repo (so far) just mimics github's codeowner review mechanism with |
|
I did notice those two repos! And I used the corpus as some ad-hoc test cases for the orphans check which ended up having a bit more nuance than this PR originally anticipated. (You can compare against the original logic in If you want to update the readme with the new command, feel free to open another PR! Otherwise I'll handle it before I publish the final 3.0.0 |
|
And |
Help remove or maintain entries that are no longer referencing existing files.