rivalcfg: init at 3.6.0 #66534

ornxka · 2019-08-12T18:56:39Z

Motivation for this change

added the rivalcfg package for configuring steelseries mice

Things done

Notify maintainers

cc @

alexarice · 2019-08-12T20:22:29Z

pkgs/misc/rivalcfg/default.nix

+    license     = licenses.wtfpl;
+  };
+
+  src = fetchurl {


I think using fetchFromGitHub is preferred

correct, although it will resolve mostly to the same thing.

alexarice · 2019-08-12T20:23:01Z

pkgs/misc/rivalcfg/default.nix

+
+  meta = with stdenv.lib; {
+    description = "Small CLI utility program that allows you to configure SteelSeries Rival gaming mice";
+    license     = licenses.wtfpl;


you should include a homepage (even if this is the same as the github url) and a maintainer

Is the maintainer me? Should I add myself to the maintainers file?

If you are happy to be the maintainer you should make a seperate commit adding yourself to the maintainers file

maintainer is someone with some working knowledge of a package, and some amount of care

alexarice · 2019-08-12T20:23:37Z

pkgs/misc/rivalcfg/default.nix

+  pname = "rivalcfg";
+  version = "3.6.0";
+
+  meta = with stdenv.lib; {


meta usually goes at the bottom, though this is just a style thing

alexarice · 2019-08-12T20:24:28Z

pkgs/misc/rivalcfg/default.nix

@@ -0,0 +1,19 @@
+{ stdenv, fetchurl, python3Packages }:


you could have lib instead of stdenv here

alexarice · 2019-08-12T20:25:11Z

pkgs/misc/rivalcfg/default.nix

@@ -0,0 +1,19 @@
+{ stdenv, fetchurl, python3Packages }:
+
+with python3Packages; buildPythonPackage rec {


I've never seen with python3Packages; used before, do you have an example where this has been used somewhere

Yes, in tools/text/invoice2data. I grepped for buildPythonPackage since I was getting an error and copied from that package. Should I be using python37Packages?

I am not sure, I am not a python expert. My query was more about using with instead of python3Packages.buildPythonPackage here and moving the with python3Packages to the buildInputs. It can be confusing to have big with clauses, for example it is not obvious that the build inputs are coming form python3Packages

python3Package, python37Packages, python3.pkgs are all aliases of each other.

alexarice · 2019-08-12T20:26:36Z

nixos/modules/programs/rivalcfg.nix

+
+    users.extraGroups.mouse.gid = config.ids.gids.mouse;
+    services.udev.extraRules = ''
+      # SteelSeries Rival


Where are you sourcing these rules from? It may be better to set up a derivation to fetch these

Those rules are from a file in the project itself, modified so that it uses the mouse group instead of just chmodding the device nodes to 0666 because that seemed like a bad idea to me. I wasn't sure how to patch it and add it to the store though (it doesn't get installed by default), so I just hardcoded it.

you could use substituteAll or a regular patch to patch it and you could maybe add to postInstall to manually install it?

I'll try that.

alexarice · 2019-08-12T20:27:13Z

pkgs/misc/rivalcfg/default.nix

+    sha256 = "78bd2ed860ad62b5bcd2fba334d8b9fda04524f7d45dd4f57dc20d9b4124110b";
+  };
+
+  doCheck = false;


do the tests not work?

They don't, it won't install with them enabled.

does it look fixable? it would be better to have the tests if they can be made to work

Probably not by me, I looked into it a bit when it was failing and had no idea what it was doing.

alexarice · 2019-08-12T21:02:28Z

maintainers/maintainer-list.nix

@@ -3947,6 +3947,10 @@
    github = "orivej";
    name = "Orivej Desh";
  };
+  ornxka = {
+    email = "ornx@protonmail.com"


jonringer

your python package should look something like this:

with python3Packages; buildPythonPackage rec {
  pname = "rivalcfg";
  version = "3.6.0";

  src = fetchFromGitHub {
    owner = "flozz";
    repo = "rivalcfg";
    rev = "v${version}";
    sha256 = "1xfaxc2qhbgi46m8ihhcgk9xfgkrvaqk0fhfav13378cihhdwwpn";
  };

  propagatedBuildInputs = [ hidapi ];

  checkInputs = [ pytest ];

  checkPhase = "pytest";

  meta = with stdenv.lib; {
    description = "Small CLI utility program that allows you to configure SteelSeries Rival gaming mice";
    license     = licenses.wtfpl;
  };
}

pkgs/misc/rivalcfg/default.nix

ornxka · 2019-08-13T21:11:45Z

Thank you all for your input on this pull request, it has been very helpful. I believe the latest version incorporates all of your feedback and I have tested it on my machine and it seems to work, including the tests. Please tell me if there are any further issues.

jonringer · 2019-08-13T21:42:02Z

I would also consolidate your commits as so:

maintainers: add ornxka # only the maintainer entry
rivalcfg: init at 3.6.0        # python package, and entry in all-packages.nix
nixos/programs/rivalcfg: init #nixos related changes

I'm not super familiar with doing nixos modules, but seems more "correct"

cc @infinisil for his nixos knowledge and a review :)

ornxka · 2019-08-13T22:23:21Z

I have consolidated the commits and fixed the commit messages.

infinisil · 2019-08-18T00:05:31Z

nixos/modules/misc/ids.nix

@@ -640,6 +640,7 @@
      cockroachdb = 313;
      zoneminder = 314;
      paperless = 315;
+      mouse = 316;


Don't assign a static id, see #65698

Here you can just not assign one and NixOS will choose a group automatically

infinisil · 2019-08-18T19:12:39Z

nixos/modules/programs/rivalcfg.nix

+    };
+
+    users.extraGroups.mouse.gid = config.ids.gids.mouse;
+    services.udev.extraRules = fileContents "${pkgs.rivalcfg}/share/rivalcfg/99-steelseries-rival.rules";


This is no good, this requires IFD to evaluate, you should use udev.packages or so instead

ornxka · 2019-08-20T04:36:24Z

I dropped the module (I figure I can just use the "users" group instead of making a new group), and updated the package so it can be used with udev.packages.

alexarice · 2019-08-20T09:07:53Z

I don't think dropping the module is necessary, but instead include a simple module which adds the package to udev.packages and environment.systemPackages

A bit like https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/programs/light.nix

stale · 2020-06-01T12:03:23Z

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the
related code and @ mention them in a comment.
Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
irc.freenode.net.

ornxka requested a review from infinisil as a code owner August 12, 2019 18:56

ofborg bot added 6.topic: nixos 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 12, 2019

alexarice reviewed Aug 12, 2019

View reviewed changes

jonringer reviewed Aug 13, 2019

View reviewed changes

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 13, 2019

jonringer reviewed Aug 13, 2019

View reviewed changes

pkgs/misc/rivalcfg/default.nix Outdated Show resolved Hide resolved

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 13, 2019

maintainers: add ornxka

21ff2fb

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 6.topic: nixos 8.has: module (update) labels Aug 13, 2019

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 13, 2019

infinisil reviewed Aug 18, 2019

View reviewed changes

infinisil requested changes Aug 18, 2019

View reviewed changes

rivalcfg: init at 3.6.0

13821cb

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 20, 2019

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

ornxka mentioned this pull request Oct 25, 2021

rivalcfg: init at 4.3.0 #142812

Closed

12 tasks

Artturin closed this Feb 2, 2023

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rivalcfg: init at 3.6.0 #66534

rivalcfg: init at 3.6.0 #66534

ornxka commented Aug 12, 2019

alexarice Aug 12, 2019

jonringer Aug 13, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

jonringer Aug 13, 2019

alexarice Aug 12, 2019

alexarice Aug 12, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

jonringer Aug 13, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

ornxka Aug 12, 2019

alexarice Aug 12, 2019

jonringer left a comment

ornxka commented Aug 13, 2019

jonringer commented Aug 13, 2019 •

edited

ornxka commented Aug 13, 2019

infinisil Aug 18, 2019 •

edited

infinisil Aug 18, 2019

ornxka commented Aug 20, 2019

alexarice commented Aug 20, 2019

stale bot commented Jun 1, 2020

		@@ -0,0 +1,19 @@
		{ stdenv, fetchurl, python3Packages }:

		with python3Packages; buildPythonPackage rec {

rivalcfg: init at 3.6.0 #66534

rivalcfg: init at 3.6.0 #66534

Conversation

ornxka commented Aug 12, 2019

Motivation for this change

Things done

Notify maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonringer left a comment

Choose a reason for hiding this comment

ornxka commented Aug 13, 2019

jonringer commented Aug 13, 2019 • edited

ornxka commented Aug 13, 2019

infinisil Aug 18, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ornxka commented Aug 20, 2019

alexarice commented Aug 20, 2019

stale bot commented Jun 1, 2020

jonringer commented Aug 13, 2019 •

edited

infinisil Aug 18, 2019 •

edited