OWNERS: add rocm team to rocm-modules#444734
Conversation
nixpkgs-ci
bot
added
10.rebuild-darwin: 0
|
Before merge, you will need to have the Nixpkgs repo added to the rocm-maintainers team. @NixOS/org can do that, I think. |
Exists so must be perms - https://github.com/orgs/NixOS/teams/rocm-maintainers |
This comment was marked as outdated.
This comment was marked as outdated.
nixpkgs-ci
bot
added
the
12.approvals: 1
LunNova
force-pushed
the
lunnova/rocm-owners
branch
from
4a846d1 to
1aaef3e
Compare
|
I think that manual step is no longer needed as NixOS/org#178 has been implemented. |
ci/OWNERS
Outdated
| @@ -190,6 +190,9 @@ nixos/modules/installer/tools/nix-fallback-paths.nix @NixOS/nix-team @raitobeza | |||
| /pkgs/top-level/release-cuda.nix @NixOS/cuda-maintainers | |||
| /pkgs/development/cuda-modules @NixOS/cuda-maintainers | |||
|
|
|||
| # ROCm | |||
| /pkgs/development/rocm-modules @NixOS/rocm-maintainers | |||
There was a problem hiding this comment.
| /pkgs/development/rocm-modules @NixOS/rocm-maintainers | |
| /pkgs/development/rocm-modules @NixOS/rocm |
Since this is a new team, we should rename it before merging, to make sure it adheres to our new guidelines:
The team name should be as short as possible; because it is nested under the maintainers group, no -maintainers suffix is needed.
There was a problem hiding this comment.
What steps should I take to address this? Separate PR to rename in team-list.nix? Commit in this PR? and then finding someone to update the name in github?
There was a problem hiding this comment.
Right this needs to be changed on the GitHub side first and then also in this PR.
@mschwaig as team maintainer should be able to do it.
There was a problem hiding this comment.
PR should be ready, assuming @mschwaig is able to rename the team
There was a problem hiding this comment.
Happy to make the change. Looks like I am able to do it. The UI warns me that this will break mentions with the old team name on GitHub.
@wolfgangwalther @LunNova is that really what we want? The team has existed since 2022, it's now new:
5cb74a9
There was a problem hiding this comment.
I'm not sure either. Did pings work before? My assumption was that they didn't, because the team was not added to the Nixpkgs repo. Maybe that assumption is wrong and this only concerned review requests, but not pings.
There was a problem hiding this comment.
Notifications seem to have worked in the past, I was notified by email for #411736's first message with a Team mention <[email protected]> CC.
There was a problem hiding this comment.
Right, sorry my bad then. We have two options here:
- stick with the current rcom-maintainers name,
- switch to rocm
We currently have one relevant PR and 9 issues open. Everyone on the team... has already been pinged because of the ping. So I'm not sure whether a dangling reference is a problem. If it is, these 10 instances could be manually fixed instead.
I'll defer to @philiptaron who initially asked for the shorter names.
LunNova
force-pushed
the
lunnova/rocm-owners
branch
from
1aaef3e to
ba05ec9
Compare
LunNova
force-pushed
the
lunnova/rocm-owners
branch
from
ba05ec9 to
3294386
Compare
nixpkgs-ci
bot
added
the
6.topic: teams
philiptaron
changed the title
rocm team to rocm-modules
nixpkgs-ci
bot
added
12.approvals: 2
wolfgangwalther
dismissed
their stale review
unblocking, but either the owners file or the team name must currently be renamed
|
I'll hit merge once someone -- @mschwaig probably -- renames the team in GitHub. |
|
I did the rename now. Thanks to everyone involved. |
4 participants
I'd like to add the ROCm team as owners for all of rocm-modules, mirroring CUDA a few lines up.
Things done
Add a 👍 reaction to pull requests you find important.