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

python3Packages.elgato: init at 2.1.1 #126047

Merged
merged 3 commits into from Jun 18, 2021
Merged

Conversation

fabaff
Copy link
Member

@fabaff fabaff commented Jun 7, 2021

Motivation for this change

Python client for Elgato Key Lights

https://github.com/frenck/python-elgato

This is a Home Assistant dependency.

  • Target Home Assistant release: current

ToDo:

  • Update component-packages
  • Enable tests in Home Assistant
Things done
  • 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 compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@r-rmcgibbo
Copy link

r-rmcgibbo commented Jun 7, 2021

Result of nixpkgs-review pr 126047 at 7360c3e run on aarch64-linux 1

2 packages built successfully:
  • python38Packages.elgato
  • python39Packages.elgato

Result of nixpkgs-review pr 126047 at 7360c3e run on x86_64-linux 1

2 packages built successfully:
  • python38Packages.elgato
  • python39Packages.elgato

@mweinelt mweinelt moved this from To do to WIP in Home Assistant Jun 11, 2021
@mweinelt
Copy link
Member

Please update this PR.

@frenck
Copy link

frenck commented Jun 12, 2021

As the author of this package, I would request not to repack my code in this case.

Thank you for honoring the authors wishes.

@NixOS NixOS locked as too heated and limited conversation to collaborators Jun 12, 2021
@lukegb
Copy link
Contributor

lukegb commented Jun 12, 2021

Locked for now. Please see #126677 (comment).

@frenck frenck mentioned this pull request Jun 14, 2021
11 tasks
@NixOS NixOS unlocked this conversation Jun 15, 2021
@frenck
Copy link

frenck commented Jun 17, 2021

As the author of the upstream package, I do not agree with this package being added.

While licensing wise I cannot stop you at this point, I think the way it is used and packaged in this project harms the user experience and thus the upstream project.

Thank you for considering.

@ofborg ofborg bot requested review from globin, mweinelt and Mic92 June 17, 2021 18:08
@jagajaga
Copy link
Member

@frenck I don't want to sound rude, but sorry, then you need to change your license. Is this truly an open source or a proprietary software indeed? There is nothing bad in adding this package to nix collection. On the contrary, it only gives it popularization, and most importantly, helps other people. People who use this package in Nix/NixOS would not disturb you in any manner related to the packaging in our system.

Home Assistant automation moved this from WIP to Reviewed Jun 17, 2021
@lukegb
Copy link
Contributor

lukegb commented Jun 18, 2021

@frenck I don't want to sound rude, but sorry, then you need to change your license. Is this truly an open source or a proprietary software indeed? There is nothing bad in adding this package to nix collection. On the contrary, it only gives it popularization, and most importantly, helps other people. People who use this package in Nix/NixOS would not disturb you in any manner related to the packaging in our system.

[This is all my personal opinion and not the position of "The NixOS Project" or anything like that]

We've had discussions about this - in a few other issues, for other HA dependencies authored by frenck; authors of software are free to express their wishes above and beyond what the license they've chosen actually requires, and as part of the OSS ecosystem it's our responsibility to listen to and consider those requests. We should remain respectful of these requests and the human beings behind them, even if we ultimately choose not to abide by them.

For better or for worse, in NixOS "we've" decided to continue to try to package this software: in NixOS, we provide a different user experience for HA than upstream provides as a result of our declarative-configuration philosophy, and as a result as responsible members of the OSS ecosystem we need to do our part in trying to reduce the support load on upstream - we expect that users of NixOS who use the HA packaging we provide will be aware of these implications (because, well, they're users of NixOS: they're probably used to things being a little different), and that there are clear escape hatches if they choose to run HA in an upstream-supportable way (using virtualisation.oci-containers to run the provided Docker container, for instance, or manually packing a venv in a way that pip will work).

The HA ecosystem is large, and the team can't possibly understand and make everything work every weird configuration that users throw at them, so I harbour no ill will towards them for wanting to reduce the scope of configurations that they're willing to support, and am extremely grateful for all the work they, and other OSS maintainers, put into providing this software for everyone to use for free.

[This is all my personal opinion and not the position of "The NixOS Project" or anything like that]

@SuperSandro2000
Copy link
Member

As the author of the upstream package, I do not agree with this package being added.

While licensing wise I cannot stop you at this point, I think the way it is used and packaged in this project harms the user experience and thus the upstream project.

Thank you for considering.

We know this. Please stop posting this under new packages or updates here. Thank you.

@SuperSandro2000 SuperSandro2000 merged commit 108e2f2 into NixOS:master Jun 18, 2021
Home Assistant automation moved this from Reviewed to Done Jun 18, 2021
@frenck
Copy link

frenck commented Jun 18, 2021

Please stop posting this under new packages or updates here. Thank you.

Thanks, I'm aware you know this, someone looking into the history might not.
Therefore I think it is important I distance myself from it.

Thanks 👍

../Frenck

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

Successfully merging this pull request may close these issues.

None yet

7 participants