tab: init at 7.1 #67965
tab: init at 7.1 #67965
Conversation
|
|
||
| checkPhase = ''make test''; | ||
|
|
||
| installPhase = '' |
There was a problem hiding this comment.
- missing "runHook preInstall" and postInstall
- the 2 lines can be combined to
install -Dm555 -t $out/bin tab - there seems to be docs the source - please install those too
There was a problem hiding this comment.
- Added hooks. Thanks.
- Yes, 2 lines can be combined, what would be an advantage of that solution, besides saving one line :) ?
- Yes, there are docs, but "make install" does not install them, and also installation instruciton says to just copy the excutable. Man page would be useful, but there isn't one. There is quite extensive help from the excutable though (
tab -h).
There was a problem hiding this comment.
Regd docs, please copy them in. It's nicer for the end user if there is something more than just a binary regardless of what upstream installs.
There was a problem hiding this comment.
Yes, 2 lines can be combined, what would be an advantage of that solution, besides saving one line :) ?
install allows you to create the directory, copy files, set permissions and set ownership all in one go which makes it much easier to read in general (not just in the context of nix) as it keeps everything that touches a certain file or set of files together in one place as opposed to spread out.
|
|
||
| meta = with stdenv.lib; { | ||
| description = "Programming language/shell calculator"; | ||
| homepage = https://tkatchev.bitbucket.io/tab/; |
There was a problem hiding this comment.
Why do you recommend quoting? Most examples at Nixpkgs Contributors Guide are unquoted.
|
Version 7.2 is available, so I am closing this PR, and submitting a new one: #71466 |
Motivation for this change
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @