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
jo: 1.1 -> 1.2 #61040
jo: 1.1 -> 1.2 #61040
Conversation
@GrahamcOfBorg build jo |
Looks fine to me, I built and tested the result and didn't encounter any issues 👍 |
It has a small test suite that can be enabled by adding Also |
@@ -1,15 +1,14 @@ | |||
{stdenv, fetchFromGitHub, autoreconfHook}: | |||
|
|||
stdenv.mkDerivation rec { | |||
name = "jo-${version}"; | |||
version = "1.1"; | |||
name = "jo-${version}"; |
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, I'm no big fan of this alignment.
- It wastes more space
- If we, for example, change name to pname, we either generate diff noise or leave it inconsistent.
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.
I will change it to pname right away and fix alignment.
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.
Builds, runs, functions as expected. Pending other reviews, this looks good.
sha256="1gn9fa37mfb85dfjznyfgciibf142kp0gisc2l2pnz0zrakbvvy3"; | ||
owner = "jpmens"; | ||
repo = "jo"; | ||
rev = "${version}"; |
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.
it's not necessary to interpolate version
rev = "${version}"; | |
rev = version; |
sha256="1gn9fa37mfb85dfjznyfgciibf142kp0gisc2l2pnz0zrakbvvy3"; | ||
owner = "jpmens"; | ||
repo = "jo"; | ||
rev = version; |
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.
also alignment here
The "src.rev" was changed because upstream tag does not have the "v" prefix for this version. A little bit of code formatting was done as well. Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
Tested locally on NixOS and darwin. |
The "src.rev" was changed because upstream tag does not have the "v"
prefix for this version.
A little bit of code formatting was done as well.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)