-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
csv-tui: init at 1.1 #334839
csv-tui: init at 1.1 #334839
Conversation
4fe8b68
to
275e5d3
Compare
05edff8
to
1182daa
Compare
Result of 1 package built:
|
If I remember correctly you need to add yourself as maintainer in a different commit. |
Thanks, i missed that. |
Result of 1 package built:
|
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.
Welcome to nixpkgs! The program seems relevant enough to include in nixpkgs. Just a few concerns below.
pkgs/by-name/cs/csv-tui/package.nix
Outdated
|
||
src = fetchFromGitHub { | ||
owner = "nathangavin"; | ||
repo = pname; |
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.
repo = pname; | |
repo = "csv-tui"; |
Avoid repo = pname;
. See nix-community/nixpkgs-lint#21
pkgs/by-name/cs/csv-tui/package.nix
Outdated
|
||
cargoHash = "sha256-wgeVcX0zSXffAuvKw2eKXC846WlC8F9UGMoxP3IXoLE="; | ||
|
||
meta = with lib; { |
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.
meta = with lib; { | |
meta = { |
Avoid with lib;
. See #292468 (comment)
pkgs/by-name/cs/csv-tui/package.nix
Outdated
cargoHash = "sha256-wgeVcX0zSXffAuvKw2eKXC846WlC8F9UGMoxP3IXoLE="; | ||
|
||
meta = with lib; { | ||
description = "A terminal based csv editor which is designed to be not a ram hog like standard csv editors, but more useful than other text editors"; |
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 terminal based csv editor which is designed to be not a ram hog like standard csv editors, but more useful than other text editors"; | |
description = "Terminal based csv editor designed to be not a ram hog like standard csv editors, but more useful than other text editors"; |
Refer to the meta.description
rules here: https://github.com/NixOS/nixpkgs/tree/master/pkgs#meta-attributes
pkgs/by-name/cs/csv-tui/package.nix
Outdated
meta = with lib; { | ||
description = "A terminal based csv editor which is designed to be not a ram hog like standard csv editors, but more useful than other text editors"; | ||
homepage = "https://github.com/nathangavin/csv-tui"; | ||
license = licenses.unfree; |
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.
Ask upstream to license this software so we can redistribute it. See: https://github.com/NixOS/nixpkgs/tree/master/pkgs#quick-start-to-adding-a-package
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.
Agh, sorry I missed too many small details when checking the derivation.
Additionally, I had to look through the source code to figure out what the keybinds were, so I don't think it's ready for general use yet. |
All keybinds are annotated in the program itself except the arrow keys. I think it's actually very easy to use. |
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.
LGTM, thanks for making the necessary changes and getting upstream to add a license.
I've used other CSV TUIs in the past and this one does seem simpler and easier to use, so let's go with it
Result of 1 package built:
|
Description of changes
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.