CONTRIBUTING: Add note about how to commit new maintainers-list.nix entry

[matthiasbeyer]

In #247972 I found that the contributing file does not say how to add a maintainer, only the manual does (as @ius said in above PR).

Maybe we can add a note here.

[Janik-Haag]

this probably relates to #245243

[ius]

Thanks, beat me to it.

Two remarks:

1. <your github handle>: just handle I think, see this remark - it doesn't have to be a GitHub username (not all GH names are valid attribute anyway, I think?)

2. Related: I wonder whether it should be explicit about submitting it as a separate commit. I think it makes sense and I see many reviewers request this, but on the other hand most committers/the GitHub default end up creating a merge commit, thus hiding said commit?

[matthiasbeyer]

Fixed what you suggested!

I will squash this if someone requests it.

[infinisil]

this probably relates to #245243

Thanks for spreading awareness, I'd love some reviews for that once it's not a draft anymore btw, feel free to subscribe to it!

That PR is still a bit too much in flux though, I don't think it should be blocking this PR here. I can rebase my PR once this one is merged (yeah it's a bit annoying to rebase when things have been moved, but I can manage 😉)

[Lord-Valen]

2. Related: I wonder whether it should be explicit about submitting it as a separate commit. I think it makes sense and I see many reviewers request this, but on the other hand most committers/the GitHub default end up creating a merge commit, thus hiding said commit?

I think it should be. Also, merge commits don't hide other commits if what you mean by "hide" is something that implies that they somehow aren't in the branch. A merge commit adds a new commit with two parent commits (the latest commit of each branch) that resolves any conflicts. Both parent commits are included in the receiving branch. You might think that merge commits hide other commits since the dates of those commits don't change and the merge commit diff shows the same changes. A merge commit can occur any amount of time after its parents, which can result in a large distance between them. With the velocity of Nixpkgs, that usually means the merge commit and one of the parents have quite a few commits between them. Here's an example: 195975b merges abc3cc0 into master on e04428a.
Right now there are 22 commits on master in the time between 195975b and abc3cc0.

If you want to see an example of a merge commit for a PR with an additional maintainers: commit, you could explore the parents of 36ea4a4.

[VuiMuich]

On my PR adding myself to maintainers #250939 I got advised to reorder the commits, so it is before the package commit. Might be worth adding a side note for this being preferred.
Idk if this technically makes any CI checks easier (fail earlier in eval maybe?), but logically it makes sense at least.

Edit: added the PR reference

[fricklerhandwerk]

@matthiasbeyer please rebase.

Implemented

[infinisil]

This is in the wrong place now, should be in maintainers/README.md

[fricklerhandwerk]

@infinisil: this section is about commit formatting. The organizational details are in the other file. I think it makes sense to have it here, and also use the opportunity to direct people to the new extended page.

[infinisil]

True there are commit conventions in CONTRIBUTING.md, but they're further down. I'd be fine with it there, but I do think it should be in maintainers/README.md in the future (and same for package commit conventions, which should be in pkgs/README.md). But the PR here currently adds it to the PR template section, which is definitely wrong.

[VuiMuich]

I agree, that it would be a better fit in the "How to create pull requests" section, probably as part of bullet 4.

Imo it would not hurt to double this small information in both the CONTRIBUTING.md and maintainers/README.md as it is easy to be overlooked.

[infinisil]

@VuiMuich I'd rather not have it be duplicated. It makes it harder to know which part should be updated, or if you don't know about both parts they could get outdated. I think it's better to improve the linking between the sections.

I agree, that it would be a better fit in the "How to create pull requests" section, probably as part of bullet 4.

Step 4 links to the commit conventions here, where there's already information about the commit message formatting, it should be added there :)

[cafkafk]

Comma nitpick but lgtm

[VuiMuich]

I can follow that "harder to update" argument very well and as I have stumbled on a bunch of such discrepancies between different docs in the bigger nix eco system. With this in mind, I agree now, to better avoid this duplication.

Also putting it as a bullet in the "commit conventions" totally makes sense.

[fricklerhandwerk]

@matthiasbeyer can you follow up with the changes? Since there seems to be a lot of energy behind this right now, if you lack time the docs team can take this over.

[matthiasbeyer]

I was traveling today, I can have a look tomorrow!

[matthiasbeyer]

Squashed the comma commit from @cafkafk and added a new one moving the text block to "commit conventions" as per request.

I can squash these commits as well, if you like!

[cafkafk]

It probably makes most sense to have just a single commit for this, instead of one adding it and another moving it.

[asymmetric]

Not sure if here or in another PR, but it would be good to also document how to add oneself to meta.maintainers, meaning just what you did in this PR:

• how the commit should be formatted
• the placement (together with a commit making a change to a package, or separately?)

Same for removal probably.

[infinisil]

@asymmetric It's mostly there in maintainers/README.md, though it could use a bit of work

[fricklerhandwerk]

Let's merge this first and follow up afterwards.