Navigation Menu

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

mullvad-vpn: init at 2019.5 #63420

Merged
merged 1 commit into from Jun 21, 2019
Merged

Conversation

lambdaloop
Copy link
Contributor

Motivation for this change

There is no package for the Mullvad VPN client currently. I use this often and perhaps others do as well.

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 nix-review --run "nix-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.

It mostly fits the CONTRIBUTING document, except that meta.maintainers is not set, as I'm not sure what to set it to.


@ghost
Copy link

ghost commented Jun 18, 2019

Very useful. Thank you.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for writing the expression @lambdaloop.
The following comments should be clarified or changes should be implemented to reflect them.

Also there might be some confusion on the package name here, in AUR it's mullvad-vpn and the actual executable for the GUI is that. So I'm thinking we should go with that name.

pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
@lambdaloop
Copy link
Contributor Author

lambdaloop commented Jun 19, 2019

Hello @worldofpeace, thank you for the comments. I've updated the expression based on your comments. It is much cleaner now and still seems to work fine. Thank you!

Please review the changes further and let me know if you have any other comments.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/mullvad/default.nix Outdated Show resolved Hide resolved
@lambdaloop
Copy link
Contributor Author

Hi @worldofpeace , thank you for the fast response. I've updated the package according to your latest comments.

Let me know how it looks.

@worldofpeace
Copy link
Contributor

LGTM @lambdaloop. We just need to squash the commits and should be able to merge ✨

@lambdaloop
Copy link
Contributor Author

@worldofpeace squashed!

@worldofpeace worldofpeace changed the title mullvad: init at 2019.5 mullvad-vpn: init at 2019.5 Jun 21, 2019
@worldofpeace worldofpeace merged commit 8c574b8 into NixOS:master Jun 21, 2019
@worldofpeace
Copy link
Contributor

Thanks @lambdaloop

@ghost
Copy link

ghost commented Jun 22, 2019

Thank you @lambdaloop and @worldofpeace.

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

3 participants