meta: add N-API to codeowners coverage

[mhdawson]

We have this guidance for contributing to N-API:
https://github.com/nodejs/node/blob/master/doc/guides/adding-new-napi-api.md

It makes sense to have one of the N-API team sign off on commits
that changes N-API

Signed-off-by: Michael Dawson [email protected]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[richardlau]

It makes sense to have one of the N-API team sign off on commits
that changes N-API

The team may have to be made a sub team of collaborators for this to work, see discussion in #33895.

[richardlau]

Should the relevant doc pages also require sign off?

[mhdawson]

@richardlau makes sense added.

[devsnek]

are these supposed to be commented out

[mhdawson]

@devsnek that is a good question. I saw most of them were commented out so followed the same pattern. @jasnell since you left many of the them commented out for now is it ok to turn it on for N-API now or do you think we should let the initial set bake for a while? Either way I'd like to add N-API now (either active or commented out) so that we don't forget.

[mmarchini]

The ones commented out are for teams which does not have write permission on this repository (teams which are not subteams of collaborators), as per discussions in #33895.

I'm working on an alternative to codeowners using the github-bot to work around GitHub's constraint on only teams with write permissions being valid owners. IMO it's best to keep these commented out for now, then we can uncomment when we move to the github-bot-based owners. Note that this PR is essentially a no-op this way though.

[mmarchini]

@nodejs/tsc (since according to CODEOWNERS changes to CODEOWNERS should add tsc for review, but apparently it didn't work)

[mcollina]

lgtm

[Trott]

It makes sense to have one of the N-API team sign off on commits
that changes N-API

The team may have to be made a sub team of collaborators for this to work, see discussion in #33895.

So someone can get write permissions to the repo by accident. We don't have any real auditing or oversight of these teams. It also means that right now, we'd have to boot a bunch of people off the team.

[mhdawson]

@Trott sounds like the same issue applies to the other commented out teams as well.

At this point I mostly want to make sure we don't forget applying to the N-API team once we figure how to make it work in general (and since its commented out like the others there would be no harm).

[mmarchini]

I'm working on an alternative to code owners that should work for us. PRs opened here and on github-bot

[Trott]

@Trott sounds like the same issue applies to the other commented out teams as well.

At this point I mostly want to make sure we don't forget applying to the N-API team once we figure how to make it work in general (and since its commented out like the others there would be no harm).

Oh, uh yeah, commented out is no problem. Sorry I missed that!

[jasnell]

Marking this 'Request changes' so it doesn't land without the suggested fixes

[mhdawson]

@james I think the suggestion was to leave commented out for now. I'll remove the dot but leave comment out until we figure out the issues that apply to all of the sections.

[jasnell]

Ok... in general, now that the paths are correct, the CODEOWNERS file is working perfectly for the QUIC files. Zero issues thus far.

[mhdawson]

Equivalent to a doc change and linters are ok so landing.

Resolved

[mhdawson]

@james just noticed you are still "changes requests" are you ok with it landing?

[jasnell]

Heh... our timing is perfect today... just resolved that :-)

[mhdawson]

Landed as bff7de3

[mmarchini]

Just FYI, the team is not being added as reviewer because it is not a subteam of collaborators (as an example: #34344)