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
xkcdpass: make available as application #85487
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.
Diff LGTM, builds cleanly via nixpkgs-review pr 85487
https://github.com/NixOS/nixpkgs/pull/85487
1 package built:
xkcdpass
@GrahamcOfBorg build xkcdpass
pkgs/top-level/all-packages.nix
Outdated
@@ -7474,6 +7474,8 @@ in | |||
|
|||
xfstests = callPackage ../tools/misc/xfstests { }; | |||
|
|||
xkcdpass = with pythonPackages; toPythonApplication xkcdpass; |
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.
Is this the right section in all-packages.nix
for this?
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.
Hmm ... maybe not.
I placed the new package by looking for the position of pwgen
(a tool serving a similar purpose) ...
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L6074-L6078
... and then scrolling forward to x...
.
However, if I search for the section titles marked by ###
, it seems that this is in section "DEVELOPMENT / EMSCRIPTEN"
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L3132-L7826
which doesn't seem like the right category.
I guess "TOOLS" would be a better fit?
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L506-L3131
@GrahamcOfBorg eval |
Your original method sounded reasonable, but I think tools would probably
be a better location. I wouldn't worry about moving the other one. It
honestly doesn't matter terribly much.
…On Tue, Apr 28, 2020, 6:49 PM Raphael Das Gupta ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkgs/top-level/all-packages.nix
<#85487 (comment)>:
> @@ -7474,6 +7474,8 @@ in
xfstests = callPackage ../tools/misc/xfstests { };
+ xkcdpass = with pythonPackages; toPythonApplication xkcdpass;
Hmm ... maybe not.
I placed the new package by looking for the position of pwgen
<https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L6075-L6077>
(a tool serving a similar purpose) ...
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L6074-L6078
... and then scrolling forward to x....
However, if I search for the section titles marked by ###, it seems that
this is in section "DEVELOPMENT / EMSCRIPTEN"
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L3132-L7826
which doesn't seem like the right category.
I guess "TOOLS" would be a better fit?
https://github.com/NixOS/nixpkgs/blob/f448ba9f6b404e2314e784a9b2d8a15c30aa3871/pkgs/top-level/all-packages.nix#L506-L3131
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#85487 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACNZYIZRNKIFSZIHNSAVLTDRO5MPDANCNFSM4MLI3P7Q>
.
|
but keep it as a lib, too, because it can be imported as a module: https://github.com/redacted/XKCD-password-generator/tree/xkcdpass-1.17.3#using-xkcdpass-as-an-imported-module
f448ba9
to
3926924
Compare
The method, yes, but I should have checked the result. 🙈
I've now rebased on current nixpkgs/pkgs/top-level/all-packages.nix Lines 502 to 3134 in 3926924
(Near the end of the section, due to alphabetical ordering.) After force-pushing that modified change: 1 package built:- xkcdpass
Yeah, left @drewrisinger Please re-review. |
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.
Diff LGTM.
Builds locally via nixpkgs-review pr 85487
.
https://github.com/NixOS/nixpkgs/pull/85487
1 package built:
xkcdpass
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
make
xkcdpass
available as application but keep it as a lib, too, because it can be imported as a module.Motivation for this change
Primary use of this Python package is as an application (CLI tool
xckdpass
).Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)Result of
nixpkgs-review
11 package built: