-
-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kubernetes-helm-mapkubeapis: init at 0.4.1 #301933
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks pretty good, but there's a few issues:
- interpolating pname (expecially into
repo
) is discouraged with lib
is a bit controversial, and can sometimes cause issues (in one case the version of a package was accidentally replaced with the version of nixpkgs itself).
Good to know - thank you. I've addressed your comments. I looked around at other recently |
f248a06
to
3783fa5
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3759 |
install -dm755 $out/${pname} | ||
mv $out/bin $out/${pname}/ | ||
install -m644 -Dt $out/${pname}/config/ config/Map.yaml | ||
install -m644 -Dt $out/${pname} plugin.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
install -dm755 $out/${pname} | |
mv $out/bin $out/${pname}/ | |
install -m644 -Dt $out/${pname}/config/ config/Map.yaml | |
install -m644 -Dt $out/${pname} plugin.yaml | |
install -dm755 $out/helm-mapkubeapis | |
mv $out/bin $out/helm-mapkubeapis/ | |
install -m644 -Dt $out/helm-mapkubeapis/config/ config/Map.yaml | |
install -m644 -Dt $out/helm-mapkubeapis plugin.yaml |
Interpolating pname
is discouraged.
3783fa5
to
00c4b59
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1604 |
@@ -14,4 +13,5 @@ | |||
|
|||
helm-unittest = callPackage ./helm-unittest.nix { }; | |||
|
|||
helm-mapkubeapis = callPackage ./helm-mapkubeapis.nix { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the file alphabetically sorted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not already sorted though hence why I added it to the bottom. Let me sort the file itself.
description = | ||
"A Helm plugin which maps deprecated or removed Kubernetes APIs in a release to supported APIs"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = | |
"A Helm plugin which maps deprecated or removed Kubernetes APIs in a release to supported APIs"; | |
description = "Helm plugin which maps deprecated or removed Kubernetes APIs in a release to supported APIs"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran it through nixpkgs-fmt
and this is what it spit out, but I presume that's not standard :-) I'll commit your suggestion. Thank you
Was looking for this plugin, but couldn't find it. I found this open PR: NixOS#217530 but it seems to have gone stale (no activity in over a year). I posted in it but figured everyone is too busy so I just decided to resubmit, addressing comments :-)
@ofborg build kubernetes-helmPlugins.helm-mapkubeapis |
Description of changes
Was looking for this plugin, but couldn't find it. I found this open PR:
#217530 but it seems to have gone
stale (no activity in over a year). I posted in it but figured everyone
is too busy so I just decided to resubmit, addressing comments :-)
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.