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

kodi-cli: Init at 1.1.1 #50892

Merged
merged 2 commits into from Dec 4, 2018
Merged

kodi-cli: Init at 1.1.1 #50892

merged 2 commits into from Dec 4, 2018

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.

worldofpeace
worldofpeace suggested changes Nov 22, 2018
View reviewed changes
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
Philipp and others added 2 commits November 30, 2018 14:05
@worldofpeace
kodi-cli: Init at 1.1.1
ee197de
@worldofpeace
kodi-cli: nitpicks (please fixup)
0da6575 
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
Reviewers

@aanderse aanderse aanderse left review comments

@symphorien symphorien symphorien left review comments

@c0bw3b c0bw3b c0bw3b left review comments

@worldofpeace worldofpeace worldofpeace approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@pstn @worldofpeace @c0bw3b @aanderse @symphorien @GrahamcOfBorg