Skip to content

ci: remove 'Notify Maintainers' section from PR template#1856

Merged
trueNAHO merged 1 commit intonix-community:masterfrom
0xda157:remove-maintainer-cc
Aug 20, 2025
Merged

ci: remove 'Notify Maintainers' section from PR template#1856
trueNAHO merged 1 commit intonix-community:masterfrom
0xda157:remove-maintainer-cc

Conversation

@0xda157
Copy link
Copy Markdown
Contributor

@0xda157 0xda157 commented Aug 19, 2025

now that we have automatic review requesting it's unnecessary


Submission Checklist

Notify Maintainers

@stylix-automation stylix-automation bot added the topic: ci /.github/ subsystem label Aug 19, 2025
Copy link
Copy Markdown
Member

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

I thought the same, but did not submit a PR because manual pinging is still desirable when more than

people should be notified, although this has AFAIK never occurred. Manual pining is still great for non-module changes or modules without explicit maintainers.

Should we keep this "Notify Maintainers" section or just require manual pinging inside the PR description, as in:

<DESCRIPTION>

CC: @awwpotato, @danth, @trueNAHO

<!-- Describe your PR above, following Stylix commit conventions. -->

---

## Submission Checklist

@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Aug 20, 2025

I thought the same, but did not submit a PR because manual pinging is still desirable when more than

people should be notified, although this has AFAIK never occurred.

This was done to prevent large treewide changes from pinging many people. If we ever get a module with more than five maintainers we can always increase the limit, and module maintainers can always be manually pinged but prompting pr authors to notify maintainers for every pr seems unnecessary.

Manual pining is still great for non-module changes or modules without explicit maintainers.

Consider pinging relevant module maintainers declared in modules/<MODULE>/meta.nix.

Consider that the "notify maintainers" section only instructs the author to ping maintainers from the /modules/ directory who are explicitly declared as a maintainer

Should we keep this "Notify Maintainers" section or just require manual pinging inside the PR description, as in:

<DESCRIPTION>

CC: @awwpotato, @danth, @trueNAHO

<!-- Describe your PR above, following Stylix commit conventions. -->

---

## Submission Checklist

I think we don't need to keep the section for the rare cases where manually pinging people is required.

Copy link
Copy Markdown
Member

@danth danth left a comment

Choose a reason for hiding this comment

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

I think it's good to simplify the contribution process as much as possible by removing this.

Having a module with more than 5 maintainers seems unlikely - and would probably indicate that some of those maintainers are no longer active.

@trueNAHO trueNAHO merged commit 2355da4 into nix-community:master Aug 20, 2025
7 checks passed
@trueNAHO trueNAHO changed the title pr_template: remove notify maintainers section ci: remove 'Notify Maintainers' section from PR template Aug 20, 2025
stylix-automation bot pushed a commit that referenced this pull request Aug 20, 2025
Link: #1856

Reviewed-by: Daniel Thwaites <danth@danth.me>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
(cherry picked from commit 2355da4)
@stylix-automation
Copy link
Copy Markdown
Contributor

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the has: port to stable This has been backported to the latest stable branch label Aug 20, 2025
trueNAHO pushed a commit that referenced this pull request Aug 20, 2025
Link: #1856

Reviewed-by: Daniel Thwaites <danth@danth.me>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
(cherry picked from commit 2355da4)
@0xda157 0xda157 deleted the remove-maintainer-cc branch August 20, 2025 17:51
trueNAHO added a commit to trueNAHO/stylix that referenced this pull request Aug 27, 2025
Remove the explicit 'Submission Checklist' header since it is the only
header in the template section after applying commits e544f6e ("ci:
visually distinguish user description from PR template content (nix-community#1802)")
and 2355da4 ("ci: remove 'Notify Maintainers' section from PR
template (nix-community#1856)").

This simplifies the template to its essentials, while keeping the
descriptive checklist code comment.
danth pushed a commit that referenced this pull request Sep 2, 2025
Remove the explicit 'Submission Checklist' header since it is the only
header in the template section after applying commits e544f6e ("ci:
visually distinguish user description from PR template content (#1802)")
and 2355da4 ("ci: remove 'Notify Maintainers' section from PR
template (#1856)").

This simplifies the template to its essentials, while keeping the
descriptive checklist code comment.

Link: #1869

Approved-by: Daniel Thwaites <danth@danth.me>
stylix-automation bot pushed a commit that referenced this pull request Sep 2, 2025
Remove the explicit 'Submission Checklist' header since it is the only
header in the template section after applying commits e544f6e ("ci:
visually distinguish user description from PR template content (#1802)")
and 2355da4 ("ci: remove 'Notify Maintainers' section from PR
template (#1856)").

This simplifies the template to its essentials, while keeping the
descriptive checklist code comment.

Link: #1869

Approved-by: Daniel Thwaites <danth@danth.me>
(cherry picked from commit 989312a)
danth pushed a commit that referenced this pull request Sep 2, 2025
Remove the explicit 'Submission Checklist' header since it is the only
header in the template section after applying commits e544f6e ("ci:
visually distinguish user description from PR template content (#1802)")
and 2355da4 ("ci: remove 'Notify Maintainers' section from PR
template (#1856)").

This simplifies the template to its essentials, while keeping the
descriptive checklist code comment.

Link: #1869

Approved-by: Daniel Thwaites <danth@danth.me>
(cherry picked from commit 989312a)
make-42 pushed a commit to make-42/stylix that referenced this pull request Sep 13, 2025
…ty#1856)

Link: nix-community#1856

Reviewed-by: Daniel Thwaites <danth@danth.me>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has: port to stable This has been backported to the latest stable branch topic: ci /.github/ subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants