haylxon: init at 1.0.0 #328651
haylxon: init at 1.0.0 #328651
Conversation
NixOSInfra
added
the
12. first-time contribution
scientiac
force-pushed
the
master
branch
2 times, most recently
from
59c8de5 to
c1f22e6
Compare
There was a problem hiding this comment.
Thanks for this, @scientiac.
Please create a single commit to add you to the maintainers list, for an example please see 59fdb6d.
Then change the order of the commits on this PR, so that the maintainers list commit comes before the init commit 🙂
Additionally please find below a few questions and suggestions for improvement.
pkgs/by-name/hx/hxn/package.nix
Outdated
| description = "Blazingly fast tool to grab screenshots of url/webpages from terminal."; | ||
| homepage = "https://crates.io/crates/hxn"; | ||
| license = licenses.mit; | ||
| maintainers = with maintainers; [ pwnwriter ]; |
There was a problem hiding this comment.
❓ Are you on-board with becoming the maintainer of this package, @pwnwriter ?
ℹ️ This was changed while writing this PR review.
pkgs/by-name/hx/hxn/package.nix
Outdated
| buildInputs = lib.optionals stdenv.isDarwin [ darwin.apple_sdk.frameworks.Security ]; | ||
|
|
||
| meta = with lib; { | ||
| description = "Blazingly fast tool to grab screenshots of url/webpages from terminal."; |
There was a problem hiding this comment.
meta.description should:
- be factual as well as short and succinct
- not end with punctuation
For further details please refer to https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#meta-attributes
| description = "Blazingly fast tool to grab screenshots of url/webpages from terminal."; | |
| description = "Save screenshots of urls and webpages from terminal"; |
pkgs/by-name/hx/hxn/package.nix
Outdated
|
|
||
| meta = with lib; { | ||
| description = "Blazingly fast tool to grab screenshots of url/webpages from terminal."; | ||
| homepage = "https://crates.io/crates/hxn"; |
There was a problem hiding this comment.
This feels like a more appropriate homepage
| homepage = "https://crates.io/crates/hxn"; | |
| homepage = "https://github.com/pwnwriter/haylxon"; |
pkgs/by-name/hx/hxn/package.nix
Outdated
There was a problem hiding this comment.
Should this package directory be named ha/haylxon?
It appears that while the program is named hxn the project is named haylxon.
There was a problem hiding this comment.
Yes, that is correct, changing the pname to haxylon would be correct. I misunderstood and named it hxn since the binary is called hxn.
There was a problem hiding this comment.
Changing pname to haxylon will result in failure because the package is called hxn on crate.io. So, hxn will be the pname not haxylon.
There was a problem hiding this comment.
❓ Out of curiosity: Could pname in fetchCrate differ from pname buildRustPackage? Not sure if it makes sense…
pkgs/by-name/hx/hxn/package.nix
Outdated
| }: | ||
|
|
||
| rustPlatform.buildRustPackage rec { | ||
| pname = "hxn"; |
There was a problem hiding this comment.
If the package directory is changed I suggest changing the package name too:
| pname = "hxn"; | |
| pname = "haylxon"; |
There was a problem hiding this comment.
Thanks for all the suggestions, all your suggestions makes sense. Yes, haxylon is the name, I thought nixpkgs required the bin name.
There was a problem hiding this comment.
Changing the name to haxylong breaks the build, hence the pname should be hxn and the by-name location must me hx/hxn.
|
Thanks for all the suggestions. I'll make all the changes you mentioned. |
github-actions
bot
added
the
8.has: maintainer-list (update)
|
I have made the suggested changes to the files. |
ofborg
bot
added
8.has: package (new)
|
Thanks for applying the changes, @scientiac, much appreciated. Looking at the commits there are still 3 instead of the expected 2 commits. Please let me know if I haven't been able to explain well how it should look in the end 🙂 |
|
Result of 1 package built:
|
Sorry for that, I didn't quite get what you meant the first time. It's fixed now. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1843 |
afh
added
the
12.approvals: 1
haylxon: remove overuse of `with lib;`
|
Thanks for merging this, @gador 🙂 👍 |
haylxon: init at 1.0.0
Description of changes
Haylxon, A tool embodying the KISS philosophy that allows you to take screenshots of webpages/URLs at lightning-fast speeds using chrome's Headless feature, means, you'd be needing a chromium based browser for it to work.
https://github.com/pwnwriter/haylxon
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.