all-maintainers.nix: init; flake/dev/generate-all-maintainers: init

[khaneliman]

Copying implementation from nix-community/home-manager#7350 with tweaks to work in nixvim. nix-community/infra#1837

[MattSturgeon]

Initial review from my phone, so I may have missed some wider context.

Looks good!

[MattSturgeon]

Sorry for dragging this through multiple review rounds. My intention was to give early feedback (earlier) and look in more detail later (now).

I have a few minor nits/questions.

Also I feel like we can further simplify the nix codegen if we can avoid needing to embed comments. I think the simplest way would be to create an attrset that represents the final value and then render that in one go using toPretty. See relevant comment(s).

Also, it'd be nice to have the file out of the top-level directory. E.g. moved to lib/ or generated/.

[zowoq]

I've renamed nixvim team to nixvim-committers and created nixvim-maintainers with triage permissions.

https://github.com/orgs/nix-community/teams/nixvim-committers
https://github.com/orgs/nix-community/teams/nixvim-maintainers

nix-community/infra#1886

Also added "Organization permissions" -> "Members" -> "Read-only" to the nixvim-ci app.

[khaneliman]

Test run https://github.com/khaneliman/nixvim/actions/runs/16123566518/job/45495054506 I don't have git config setup for PR on fork atm But, should work when the user-info step returns the app information, like in HM. Something still isn't right with generating the file in this refactoring here, though.. taking a look.

[khaneliman]

https://github.com/khaneliman/nixvim/actions/runs/16123952334/job/45496299935 alright, have to capture the output from validation.

[khaneliman]

Alright, after doing some testing https://github.com/khaneliman/nixvim/actions/runs/16126857467 I made it work better with existing PRs

[MattSturgeon]

No issues with the diff from me. And I trust your testing process.

Minor comments below, all either trivial or subjective. They can all be ignored if preferred.

[MattSturgeon]

FYI gh api also has a --jq flag:

Suggested change

	' | jq --raw-output '
	' --jq '

Sometimes it's still worth it to pipe to jq though, e.g. if you want to use --exit-status to trigger a failure if the jq query doesn't match the input JSON.

---

We use a similar graphql query elsewhere, but one issue I'm now spotting is that headRefName could also match PRs on forks that use the same branch name... I don't see an obvious quick fix in the graphql explorer, and the issue applies to both places, so this is fine for now.

[MattSturgeon]

Nit: this makes it sound like this is the canonical place where the permissions are defined. However we actually have them in two places, and this is the less important of the two since it is only used on forks that don't have an APP_ID.

[khaneliman]

Not sure why this would be less important? These are required for a token to get generated with permissions required, the others are just to limit the scope of permissions granted to an app.

[MattSturgeon]

These dictate the permissions of github.token and are entirely unrelated to the app token.

I said they're less important because github.token is only used when there's no app token.

[khaneliman]

These dictate the permissions of github.token and are entirely unrelated to the app token.

the others are just to limit the scope of permissions granted to an app.

Understood, as part of previous response.

Guess its differing opinion on what's important. I think declaring the app token permissions as less because they are optional for functionality. But, the app token permissions are just a couple lines lower with the exact same naming convention and didn't think it warranted copy/paste for same explanation.