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

pythonPackages.haas-nabucasa: init at 0.29 #75876

Merged
merged 2 commits into from Dec 26, 2019
Merged

Conversation

Scriptkiddi
Copy link
Contributor

@Scriptkiddi Scriptkiddi commented Dec 18, 2019

Motivation for this change
Things done
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

Copy link
Contributor

@jonringer jonringer left a 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
has tests

[4 built, 2 copied (0.0 MiB), 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/75876
2 package were built:
python37Packages.snitun python38Packages.snitun

just some small suggestions

pkgs/development/python-modules/snitun/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/snitun/default.nix Outdated Show resolved Hide resolved
@jonringer jonringer changed the title Py3 snitun pythonPackages.snitun: init at 0.20 Dec 18, 2019
@jonringer
Copy link
Contributor

@GrahamcOfBorg build python37Packages.snitun python38Packages.snitun

@Scriptkiddi
Copy link
Contributor Author

@jonringer I added another package to this pr since it depends on the original one, or should I open a new pull request for the next one?

@jonringer jonringer changed the title pythonPackages.snitun: init at 0.20 pythonPackages.haas-nabucasa: init at 0.29 Dec 19, 2019
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

your commit history should be:

pythonPackages.snitun: init at 0.20
pythonPackages.haas-nabucasa: init at 0.29

I would recommend using git rebase -i

@jonringer
Copy link
Contributor

it's fine, just need to update your PR to describe what you're doing

@jonringer
Copy link
Contributor

you have snitun changes in the pythonPackages.haas-nabucasa commit do the following to fix:

git reset HEAD~2 # un-commit last 2 commits, and put in unstaged
git add pkgs/development/python-modules/snitun/default.nix
git commit -m "pythonPackages.snitun: init at 0.20"
git add pkgs/development/python-modules/snitun/default.nix
git commit -m "pythonPackages.haas-nabucasa: init at 0.29"
git push <fork> <branch> --force

@marsam marsam merged commit 45e69f1 into NixOS:master Dec 26, 2019
@Scriptkiddi Scriptkiddi deleted the py3_snitun branch December 27, 2019 10:37
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jan 2, 2020
pythonPackages.haas-nabucasa: init at 0.29

(cherry picked from commit 45e69f1)
@flokli
Copy link
Contributor

flokli commented Jan 8, 2020

@Scriptkiddi can this be moved inside the pkgs/servers/home-assistant folder?

As elaborated in NabuCasa/hass-nabucasa#119 (comment), upstream "doesn't care about other ways to run Home Assistant" (except docker and virtualenv install), so we should probably not have it in the curated python package set, but move it to https://github.com/NixOS/nixpkgs/blob/master/pkgs/servers/home-assistant/default.nix, (with some overrides, so this module doesn't block acme version bumps).

@Scriptkiddi
Copy link
Contributor Author

I will do that, I will also see if we can not remove such a module upstream from the hass cloud module so its not required, lets see how good my chances are seeing that those guys from nabucasa started hass ....

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