-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
snx-rs: 2.2.3 -> 2.9.0 #376344
base: master
Are you sure you want to change the base?
snx-rs: 2.2.3 -> 2.9.0 #376344
Conversation
Fixed formatting (with nixfmt-rfc-style). Changed libsoup dependency to libsoup_3.
|
|
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.
Seems good.
@Shavyn This package is currently unmaintained ( I'd also suggest setting up automatic updates so @r-ryantm will automatically generate PRs and notify the maintainer(s) |
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.
Before merging, the commit history should be definitely cleaned up: squashing the commits, removing the merged from ...
commits, ideally keeping only one commit, which can then have additional information in the commit long description part from the current commits, if you want.
|
I looked at the upstream repo. They have a changelog file in there. We should add it to the changelog = "https://github.com/ancwrd1/snx-rs/blob/v${version}/CHANGELOG.md"; Could you make the change while you are at it, too? |
Since this project has regular releases on GitHub, adding an update script should be as easy as adding
|
checkFlags = [ | ||
"--skip=platform::linux::net::tests::test_default_ip" | ||
"--skip=platform::linux::resolver::tests::test_detect_resolver" |
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 would be good to add an explanation (a comment) here for why the test fails.
Is it something that should be reported upstream?
Thanks for all the inputs, as soon as I come back from work I'll have everything (hopefully) sorted. |
This is only a version bump of the snx-rs package as the version in nixpkgs (2.2.3) didn't support a feature added later I needed.
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.