-
-
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
mailhog: 1.0.0 -> 1.0.1 #97445
mailhog: 1.0.0 -> 1.0.1 #97445
Conversation
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.
- Diff LGTM
- Commits LGTM
- Builds via
nix-review
- Missing changelog
meta.changelog
- Didn't/don't know how to run the test.
https://github.com/NixOS/nixpkgs/pull/97445
1 package built:
mailhog
975b266
to
ed4a618
Compare
@drewrisinger the test can be run with |
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.
Small change to Diff about fetcher rev, then LGTM.
I tried to run the test, but got the following error, not worth debugging IMHO. Would appreciate a merger to look over the test before merging, b/c I have no experience w/ nixos tests
error: a 'x86_64-linux' with features {kvm, nixos-test} is required to build '/nix/store/s52b23f87zr48j8h6srbiq5z09i17blk-vm-test-run-mailhog.drv', but I am a 'x86_64-linux' with features {benchmark, big-parallel, nixos-test}
@@ -2,7 +2,7 @@ | |||
|
|||
buildGoPackage rec { | |||
pname = "MailHog"; | |||
version = "1.0.0"; | |||
version = "1.0.1"; | |||
rev = "v${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.
rev = "v${version}"; |
This is non-standard. This line should be kept, moved into fetchFromGitHub.
ed4a618
to
998df28
Compare
Was just browsing through the meta section of manual, and that might suggest adding I've honestly never dealt with nixos tests, so sorry I'm not much help. |
998df28
to
d4a45be
Compare
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.
The commit should be separated into two - one adding a test and the other for the update.
d4a45be
to
df7a435
Compare
Result of 1 package built:
|
Test runs fine as well. |
Motivation for this change
New upstream release https://github.com/mailhog/MailHog/releases/tag/v1.0.1
Also added a simple test.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)