Skip to content

ci: tag maintainers for review#6921

Merged
khaneliman merged 1 commit intonix-community:masterfrom
khaneliman:maintainers
Jul 2, 2025
Merged

ci: tag maintainers for review#6921
khaneliman merged 1 commit intonix-community:masterfrom
khaneliman:maintainers

Conversation

@khaneliman
Copy link
Copy Markdown
Collaborator

Description

Checklist

  • Change is backwards compatible.

  • Code formatted with nix fmt or ./format.

  • Code tested through nix-shell --pure tests -A run.all
    or nix build --reference-lock-file flake.lock ./tests#test-all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

@khaneliman khaneliman force-pushed the maintainers branch 4 times, most recently from 4cf43bf to 8128587 Compare April 28, 2025 01:02
@khaneliman
Copy link
Copy Markdown
Collaborator Author

khaneliman commented Apr 28, 2025

@rycee I think our github token needs.

"Pull requests" repository permissions (write)

I don't have access to see the settings on the repo, though.

Comment thread .github/workflows/tag-maintainers.yml Outdated
Comment on lines +55 to +67
for PATH_FORMAT in \
"$FILE" \
"/Users/khaneliman/github/home-manager/$FILE" \
"/$PWD/$FILE" \
"$PWD/$FILE"
do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this just for figuring out which path to use while developing the workflow?

Comment thread .github/workflows/tag-maintainers.yml Outdated
echo "Trying path format: $PATH_FORMAT"

# Get maintainers for this file using the current path format
MAINTAINERS=$(jq -r ".[\"$PATH_FORMAT\"] | if . != null then .[].github else empty end" <<< "$MODULE_MAINTAINERS" 2>/dev/null || true)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be possible to make jq read the file directly?

Suggested change
MAINTAINERS=$(jq -r ".[\"$PATH_FORMAT\"] | if . != null then .[].github else empty end" <<< "$MODULE_MAINTAINERS" 2>/dev/null || true)
MAINTAINERS=$(jq -r ".[\"$PATH_FORMAT\"] | if . != null then .[].github else empty end" ./result 2>/dev/null || true)

@rycee
Copy link
Copy Markdown
Member

rycee commented May 2, 2025

Looks nice! About the permissions I'm not certain. From what I could see in the GitHub documentation it should be possible to add a permissions field in the yaml file. Something like

permissions:
  contents: read
  pull-requests: write

perhaps. This seems to be used by the eval flow in Nixpkgs: https://github.com/NixOS/nixpkgs/blob/d66115b18ccf2fbb03da4f2ea8a41499eb8d3136/.github/workflows/eval.yml#L241-L243

@khaneliman
Copy link
Copy Markdown
Collaborator Author

Looks nice! About the permissions I'm not certain. From what I could see in the GitHub documentation it should be possible to add a permissions field in the yaml file. Something like

permissions:
  contents: read
  pull-requests: write

perhaps. This seems to be used by the eval flow in Nixpkgs: https://github.com/NixOS/nixpkgs/blob/d66115b18ccf2fbb03da4f2ea8a41499eb8d3136/.github/workflows/eval.yml#L241-L243

I'm wondering if this is related to something I've seen previously, github wont let us request reviews from those who aren't collaborators. In nixpkgs, we add everyone to the Nixos organization. We might not be able to do this here, then....

https://github.com/NixOS/nixpkgs/blob/d66115b18ccf2fbb03da4f2ea8a41499eb8d3136/ci/request-reviews/request-reviewers.sh#L61-L69

Tested with this block in there and that seems to be the case

@github-actions github-actions bot added the window-managers sway, gnome, i3 label Jun 24, 2025
@khaneliman khaneliman force-pushed the maintainers branch 9 times, most recently from dbe8d01 to e557a79 Compare July 2, 2025 16:18
@khaneliman khaneliman marked this pull request as ready for review July 2, 2025 16:22
@khaneliman khaneliman marked this pull request as draft July 2, 2025 16:23
@khaneliman khaneliman marked this pull request as ready for review July 2, 2025 16:34
@khaneliman khaneliman changed the title tag-maintainers.yml: init ci: tag maintainers for review Jul 2, 2025
@khaneliman khaneliman marked this pull request as draft July 2, 2025 16:35
@khaneliman khaneliman marked this pull request as ready for review July 2, 2025 16:45
@khaneliman khaneliman marked this pull request as draft July 2, 2025 16:46
@khaneliman khaneliman force-pushed the maintainers branch 2 times, most recently from 2c4c045 to d46842a Compare July 2, 2025 17:30
@khaneliman khaneliman marked this pull request as ready for review July 2, 2025 17:32
@github-actions github-actions bot removed the window-managers sway, gnome, i3 label Jul 2, 2025
@khaneliman
Copy link
Copy Markdown
Collaborator Author

khaneliman commented Jul 2, 2025

Using PR target I need to check this in to really test it. But from a test in my repo with https://github.com/khaneliman/home-manager/actions/runs/16031756101/job/45233835846?pr=567 it appeared to make it down to requesting a review but failed because it was looking against my fork and not this repo.

EDIT: Still have some cleanup I want to do

@khaneliman khaneliman force-pushed the maintainers branch 2 times, most recently from 36d7f67 to d12124f Compare July 2, 2025 18:46
@khaneliman
Copy link
Copy Markdown
Collaborator Author

https://github.com/khaneliman/home-manager/actions/runs/16033202876/job/45238595516?pr=568 looks even better. Was able to test simplifying some logic and handling people who have already been reviewing so they dont get re-requested.

@khaneliman khaneliman marked this pull request as draft July 2, 2025 18:48
@khaneliman khaneliman marked this pull request as ready for review July 2, 2025 18:48
@khaneliman
Copy link
Copy Markdown
Collaborator Author

Would like to test against someone invited to my fork as a collaborate to make sure that flow works to avoid a bunch of fixes PRs.

Want to create an easier way to notify maintainers that someone is
working on their module.

Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
@khaneliman
Copy link
Copy Markdown
Collaborator Author

Sweet, got it working khaneliman#568

@khaneliman khaneliman merged commit 25f003f into nix-community:master Jul 2, 2025
4 checks passed
@khaneliman khaneliman deleted the maintainers branch July 2, 2025 19:45
khaneliman added a commit that referenced this pull request Jul 31, 2025
Want to create an easier way to notify maintainers that someone is
working on their module. Added a workflow for requesting a review from any maintainers that have joined the `home-manager-maintainers` team in the organization. 

Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
khaneliman added a commit to khaneliman/home-manager that referenced this pull request Sep 1, 2025
Want to create an easier way to notify maintainers that someone is
working on their module. Added a workflow for requesting a review from any maintainers that have joined the `home-manager-maintainers` team in the organization. 

Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
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.

2 participants