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.

Sign up for GitHub

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

Jump to bottom

nixos/unified-remote: init at 3.6.0.745 #30668

Closed
wants to merge 2 commits into from
Closed

nixos/unified-remote: init at 3.6.0.745 #30668

wants to merge 2 commits into from

Conversation

badmutex
Copy link
Contributor

Motivation for this change

Hi

I've used Unified Remote to control my media center PC from an old phone for years now.
After switching the OS to NixOS, I needed to run something similar.
A superficial search didn't turn up a comparable service already in NixOS and everyone in the house is already comfortable with Unified Remote, hence this PR.

Example usage:
  1. Enable the service 
    services.unified-remote.enable = true;
services.unified-remote.openFirewall = true;
  2. Install app on phone
Things done
  1. Adds the unified-remote package
  2. Adds a unified-remote service
  3. Adds nixos test
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.
    I think. This is both package and nixos/service.

@globin
Copy link
Member

globin commented Oct 21, 2017

Please don't PR to release branches, go through master and request a cherry-pick after.

Also please squash your commits into logical chunks.

@globin globin changed the base branch from release-17.09 to master October 21, 2017 22:27
@badmutex
Copy link
Contributor Author

@globin sorry about that. I made some changes. Do they address your concerns?

@badmutex
Copy link
Contributor Author

Travis fails due to the unfree license.

edolstra
edolstra reviewed Oct 23, 2017
View reviewed changes
nixos/modules/services/web-apps/unified-remote.nix Outdated
example = true;
description = ''
Whether to enable Unified Remote Server

Copy link
Member

Choose a reason for hiding this comment

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

Note that all this formatting will not show up in the manual.

edolstra
edolstra reviewed Oct 23, 2017
View reviewed changes
nixos/modules/misc/ids.nix Outdated
@@ -567,6 +567,7 @@
kanboard = 281;
pykms = 282;
kodi = 283;
uinput = 284;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to assign a fixed uid?

edolstra
edolstra reviewed Oct 23, 2017
View reviewed changes
nixos/modules/services/web-apps/unified-remote.nix Outdated
stateDir = mkOption {
type = types.path;
default = "/var/lib/unified-remote";
description = "Path to the state dir";
Copy link
Member

Choose a reason for hiding this comment

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

"dir" is not a word. Also, sentences should end with a period.

@edolstra
Copy link
Member

I'm not really in favor of adding unfree software to NixOS unless there is a compelling reason.

@badmutex
Copy link
Contributor Author

Thank you for your comments @edolstra. I appreciate your concern and would be happy continuing to maintain these patches out of tree for now.

There are a couple flaws with the PR:

  1. The unfree license
  2. Opacity of the upstream update process

That said, my experience using NixOS plus this service for a couple months has been positive.
A single data point is probably not a good enough reason to merge though.

@badmutex
Copy link
Contributor Author

badmutex commented Jan 1, 2018

Closing this as I've moved the module and package elsewhere https://gitlab.com/badi.sh/nixos-mediacenter

@badmutex badmutex closed this Jan 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra edolstra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@badmutex @globin @edolstra