Skip to content
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

nicotineplus: init at 1.4.1 #53861

Closed
wants to merge 6 commits into from
Closed

nicotineplus: init at 1.4.1 #53861

wants to merge 6 commits into from

Conversation

6AA4FD
Copy link
Contributor

@6AA4FD 6AA4FD commented Jan 13, 2019

Added myself (6AA4FD) to maintainer list

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Contributor

Also, if you intend to submit other prs in the future, they should be checked out to a branch from master and not on master itself.

Other than that it's nice to see our maintainers list grow ✨

@6AA4FD
Copy link
Contributor Author

6AA4FD commented Jan 13, 2019

alright. everything looks good on my end, and i'll use branches on my fork instead of master for future commits.

@worldofpeace
Copy link
Contributor

Builds locally and executes, though there is this warning

Nicotine+ supports a country code blocker. This requires a (GPL'ed)
          library called GeoIP. You can find it here: C library:
          http://www.maxmind.com/app/c Python bindings:
          http://www.maxmind.com/app/python (the python bindings require the C
          library)

Please squash your commits into one with a message identical to your pr title aside from the changed attribute nicotine-plus.

@6AA4FD
Copy link
Contributor Author

6AA4FD commented Jan 14, 2019

i'm happy to do that, but i've just realized that #53717 predates this, and seems to be better put together, so perhaps that should be tested+merged instead?

@worldofpeace
Copy link
Contributor

i'm happy to do that, but i've just realized that #53717 predates this, and seems to be better put together, so perhaps that should be tested+merged instead?

Looks like that was merged. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants