Skip to content

Conversation

@MattSturgeon
Copy link
Contributor

@MattSturgeon MattSturgeon commented Sep 10, 2025

Many inactive committers seem confused by their retirement PRs, especially if they have been active maintainers or have actively contributed in other ways. For example: #26 (comment)

Hopefully, clearly stating which activities require commit access and which do not, will help avoid such confusion.

It's also likely that many are already aware of this. Even so, I think highlighting it within the retirement PR will help them focus their arguments on relevant facts, instead of highlighting irrelevant facts not related to committing.

I've implemented this as a "note" admonition, however other possible admonitions supported by GFM include "tip", "important", "warning", and "caution".

I believe this would be valid as a "note", "tip", or "important" admonition.

Feedback on my wording, implementation, formatting, etc is welcome; especially if it leads to a better PR description template in the end.

Examples

Note

Commit access is not needed for most forms of contributing, including being a maintainer and reviewing PRs. It is only needed for things that requires write permissions to nixpkgs, such as merging PRs.

Tip

Commit access is not needed for most forms of contributing, including being a maintainer and reviewing PRs. It is only needed for things that requires write permissions to nixpkgs, such as merging PRs.

Important

Commit access is not needed for most forms of contributing, including being a maintainer and reviewing PRs. It is only needed for things that requires write permissions to nixpkgs, such as merging PRs.

Aside

In the original issue discussing this automation, it was mentioned that these PRs would give committers one month to make a commit:

When the workflow runs the next month, any existing PRs will either be merged or closed automatically based on whether the committer has used their commit access or not.

(emphasis mine)

However, the current wording instead talks about making a comment within the month:

Make a comment with your motivation to keep commit access, otherwise this PR will be merged and implemented in 1 month.

Is this a deliberate wording choice, to avoid committers rushing to make poor-quality commits during their probationary month?

Even so, I'm slightly uncomfortable if there is a policy in place that isn't being communicated within these PRs (intentionally or not).

@MattSturgeon MattSturgeon requested a review from a team September 10, 2025 21:28
@emilazy
Copy link
Member

emilazy commented Sep 10, 2025

such as merging PRs

Technically: PRs that can’t be merged with the merge bot, i.e. PRs other than ones by r-ryantm or committers, or that touch files that aren’t by-name packages you maintain. (I know you know this, of course, and it may not be that the wording needs adjusting for it, but just pointing it out in case there is a graceful way to accommodate it.)

Many inactive committers seem confused by their retirement PRs,
especially if they _have_ been active maintainers or have actively
contributed in other ways.

Hopefully, clearly stating which activities require commit access and
which do not, will help avoid such confusion.
@MattSturgeon
Copy link
Contributor Author

such as merging PRs

Technically: PRs that can’t be merged with the merge bot

Yes, you're correct to point this out. I had the same thought myself.

Personally, I was already struggling to strike a balance between giving concrete examples and not rambling on too much... Therefore, I felt caveating "you need commit access to merge PRs manually, but sometimes you can ask the merge bot to do it for you" would've been a bit too wordy.

But if we can find a sensible way to mention the merge bot, maybe that'll satisfy maintainers who just want to be able to merge r-ryantm update PRs?

I'm open to suggestions, but I'm also happy to postpone it for another PR. I don't think it's worth mentioning if we can't find a way to do so without distracting from the key point.

Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not part of the delegator team, but I don't think this needs their approval, so I'll just merge it :)

@infinisil infinisil merged commit 297f123 into NixOS:main Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants