ani-skip: init at 1.0.1 #339027
ani-skip: init at 1.0.1 #339027
Conversation
|
Suggestions instead of the sed line are especially welcome, I find it ugly (it's for adding to a specific part of the script, hence the backreference referencing the entire lhs). |
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
and removed
10.rebuild-darwin: 0
labels
| runtimeInputs = [ | ||
| gnugrep | ||
| gnused | ||
| curl | ||
| fzf | ||
| ]; | ||
|
|
There was a problem hiding this comment.
| runtimeInputs = [ | |
| gnugrep | |
| gnused | |
| curl | |
| fzf | |
| ]; |
pkgs/by-name/an/ani-skip/package.nix
Outdated
| --replace-fail '--script-opts=%s' "--script=$out/share/mpv/scripts/skip.lua --script-opts=%s" | ||
|
|
||
| wrapProgram $out/bin/ani-skip \ | ||
| --suffix PATH : ${lib.makeBinPath finalAttrs.runtimeInputs} |
There was a problem hiding this comment.
| --suffix PATH : ${lib.makeBinPath finalAttrs.runtimeInputs} | |
| --suffix PATH : ${lib.makeBinPath [ gnugrep gnused curl fzf ]} |
grep and sed are defaults so we don't necessarily need them
There was a problem hiding this comment.
What's the point of this change? Why not just have them as an attribute for flexibility with overrides?
There was a problem hiding this comment.
Very nit but this is what id do:
{
...
extraRuntimeInputs ? [],
}:
stdenv.mkDerivation {
...
wrapProgram --suffix PATH ${lib.makeBinPath ([ curl fzf ] ++ extraRuntimeInputs)}
...
}
Very nit and opinionated, the way youve done it is in my opinion perfectly acceptable
There was a problem hiding this comment.
I'd rather keep the current implementation. Introducing an argument is overkill.
There was a problem hiding this comment.
Actually this is better than what you suggests, because it allows overriding the list easily. Also we can't guarantee our programs don't run on other systems and external grep or sed is a source of impurity.
|
@SuperSandro2000 any chance we could make progress on this (the remaining CI action seems to be stuck, and there are no tests anyway)? If you're not convinced by my message above, imagine the following scenario (which has happened to me multiple times with other packages): a user wants a new commit from upstream, but they haven't done a release with it, and the nixpkgs package follows its release cycle, so they have to override the src themself. If there were dependencies introduced since the last release, they would have to add those too, and if the dependencies are hardcoded, instead of being an attribute, the user would have to override the postPatch phase (copying and modifying the original), which, in my opinion, is bad. |
Description of changes
Add https://github.com/synacktraa/ani-skip.
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.