Skip to content

kodi-cli: Init at 1.1.1#50892

Merged
c0bw3b merged 2 commits intoNixOS:masterfrom
pstn:kodi-cli
Dec 4, 2018
Merged

kodi-cli: Init at 1.1.1#50892
c0bw3b merged 2 commits intoNixOS:masterfrom
pstn:kodi-cli

Conversation

@pstn
Copy link
Contributor

@pstn pstn commented Nov 21, 2018

Motivation for this change

Added the handy utility kodi-cli for controlling remote kodi instances with a cli.
I was on the fence myself about PRing this, so I asked in #nixos. A few people wanted to have it, so here it is.

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)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 21, 2018
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.

A lot of the indentation and whitespace isn't up to par with what's in nixpkgs.
See: https://nixos.org/nixpkgs/manual/#sec-syntax

@pstn
Copy link
Contributor Author

pstn commented Nov 22, 2018

Whoopsie. Made a little git mess. Please disregard until I clean it up.

@pstn pstn force-pushed the kodi-cli branch 2 times, most recently from 4b9a6ff to c70333e Compare November 22, 2018 22:59
pstn and others added 2 commits November 30, 2018 14:05
nitpicks applied are:

  - The pname thing
    staging-next has been merged.

  - Moved to tools/misc
    applications/video is more appropriate for video applications.
    This is a script used to interact with one.

  - Changed platforms to unix
    This script can only be used where kodi is present.
@worldofpeace
Copy link
Contributor

@pstn I've pushed basically reviewer nitpicks to your branch. I hope you don't mind.

Also, my apologies for your change being shelved on this pname discussion.
IMHO that shouldn't have been important as that change wasn't even on master yet.

@c0bw3b
Copy link
Contributor

c0bw3b commented Dec 1, 2018

@GrahamcOfBorg build kodi-cli

@pstn
Copy link
Contributor Author

pstn commented Dec 1, 2018

@worldofpeace All good! I actually made the same point about waiting with a merge for this package myself. I also like the nitpicks, just had a busy week and no time to merge.

@c0bw3b c0bw3b merged commit 169e279 into NixOS:master Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants