(jira-cli) init at 2.2 #30833

nyarly · 2017-10-26T17:34:28Z

No description provided.

FRidh · 2017-10-26T18:52:16Z

What is the reason for not using the packages in python-packages.nix?

nyarly · 2017-10-26T19:04:15Z

There's no reason per se. I just used pypi2nix to build this - it's my first python nix expression.

Can you advise on how to use the python packages instead? My impression is of a lot of effort - is there gain to be had?

nyarly · 2017-11-22T18:04:19Z

Ping on this PR?

FRidh · 2017-11-25T09:05:57Z

pyp2nix can be used in Nixpkgs as a last resort. Otherwise the common packages should be used.

matthewbauer

See @FRidh's comments.

nyarly · 2018-04-21T23:35:09Z

@matthewbauer @FRidh I'll take a crack at the conversion.

nyarly · 2018-04-22T03:02:56Z

@matthewbauer @FRidh I spent some time with this but am now stymied by Could not find a version that satisfies the requirement argparse; python_version < "3.2" (from jira==1.0.10) (from versions: ).

I see that argparse is nulled out intentionally in the pythonPackages expression, so I'm not sure how to proceed.

FRidh · 2018-04-22T06:31:50Z

@nyarly argparse is part of the standard library of Python since 2.7. There is no reason for packages to any longer depend on it. The solution here is to patch the (typically) setup.py file to remove that dependency.

nyarly · 2018-04-22T22:10:44Z

Okay, this is working with the distro python packages now.

FRidh

The comments apply also to the other expressions.

And please read the contribution guidelines regarding Python packages.

FRidh · 2018-04-23T05:53:04Z

pkgs/top-level/python-packages.nix

@@ -5097,6 +5097,26 @@ in {
    };
  };

+  keyrings = {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

FRidh · 2018-04-23T05:54:01Z

pkgs/top-level/python-packages.nix

@@ -5097,6 +5097,26 @@ in {
    };
  };

+  keyrings = {
+    alt = buildPythonPackage rec {


This doesn't look good. It's supposed to be keyrings-alt. Don't nest it just so you get a dot.

FRidh · 2018-04-23T05:54:08Z

pkgs/top-level/python-packages.nix

+    alt = buildPythonPackage rec {
+      name = "keyrings.alt-${version}";
+      version = "2.3";
+      src = pkgs.fetchurl {


The fetchurl from the mirror was in imitation of other expressions here. I'll happily change mine, but can I suggest that should be done across the python-packages modules?

FRidh · 2018-04-23T05:54:26Z

pkgs/top-level/python-packages.nix

+      propagatedBuildInputs = [ self.six ];
+
+      #
+      doCheck = false;


Why are the tests disabled? Please include a comment.

In every case it was "I couldn't get the tests to run and pass." In no small part this is that Python isn't one of my working languages, and hunting down "how does this package run tests" is challenging.

FRidh · 2018-04-23T05:54:39Z

pkgs/top-level/python-packages.nix

+      #
+      doCheck = false;
+
+      meta = with pkgs.stdenv.lib; {


maintainer?

I have no idea about the homepage. I assume pypi2nix would've added that if it were available from pypi.

As to maintainer: should I add myself?

FRidh · 2018-04-23T05:55:40Z

pkgs/development/tools/jira_cli/default.nix

@@ -0,0 +1,31 @@
+{ pkgs, stdenv, pythonPackages }:


Don't pass in pkgs, only the exact packages you need.

FRidh · 2018-04-23T05:56:05Z

pkgs/development/tools/jira_cli/default.nix

+  inherit (pkgs) libffi openssl;
+  inherit (pythonPackages) fetchPypi buildPythonPackage;
+in
+  buildPythonPackage rec {


This is an application, use buildPythonApplication.

FRidh · 2018-04-23T05:56:13Z

pkgs/development/tools/jira_cli/default.nix

+      substituteInPlace requirements/main.txt --replace "argparse" ""
+    '';
+
+    doCheck = false;


Why are the tests disabled? Please include a comment.

nyarly · 2018-04-23T19:03:16Z

@FRidh Re: the python contribution guide, I assume you mean https://nixos.org/nixpkgs/manual/#python? I had, and was under the impression I was following those guidelines. There are certainly errors in this PR as is, which I'm working on.

looks good to me

FRidh · 2018-04-25T05:31:02Z

pkgs/top-level/python-packages.nix

@@ -2993,6 +2993,23 @@ in {

  };

+  hiro = buildPythonPackage rec {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

Aha - I'd misunderstood as applying to the keyrings set until you'd realized that alt was only a child to get the .. I'll take this on presently.

FRidh · 2018-04-25T05:31:09Z

pkgs/top-level/python-packages.nix

@@ -5097,6 +5114,29 @@ in {
    };
  };

+  keyrings-alt = buildPythonPackage rec {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

FRidh · 2018-04-25T05:31:17Z

pkgs/top-level/python-packages.nix

@@ -5097,6 +5114,29 @@ in {
    };
  };

+  keyrings-alt = buildPythonPackage rec {
+    name = "${pname}-${version}";


The name attribute is added by buildPython* and should therefore be removed.

FRidh · 2018-04-25T05:31:47Z

pkgs/top-level/python-packages.nix

+
+    # Fails with "ImportError: cannot import name mock"
+    doCheck = false;
+    #checkInputs = with self; [ pytest unittest2 mock keyring ];


This can be uncommented; they will be ignored when doCheck = false;

FRidh · 2018-04-25T05:31:54Z

pkgs/top-level/python-packages.nix

@@ -6880,6 +6920,34 @@ in {

  jinja2_pluralize = callPackage ../development/python-modules/jinja2_pluralize { };

+  jira = buildPythonPackage rec {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

FRidh · 2018-04-25T05:31:58Z

pkgs/top-level/python-packages.nix

@@ -6880,6 +6920,34 @@ in {

  jinja2_pluralize = callPackage ../development/python-modules/jinja2_pluralize { };

+  jira = buildPythonPackage rec {
+    name = "${pname}-${version}";


The name attribute is added by buildPython* and should therefore be removed.

FRidh · 2018-04-25T05:32:07Z

pkgs/top-level/python-packages.nix

@@ -7742,6 +7810,25 @@ in {
    };
  };

+  requests-oauthlib = buildPythonPackage rec {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

FRidh · 2018-04-25T05:32:20Z

pkgs/top-level/python-packages.nix

@@ -7742,6 +7810,25 @@ in {
    };
  };

+  requests-oauthlib = buildPythonPackage rec {
+    name = "${pname}-${version}";


The name attribute is added by buildPython* and should therefore be removed.

FRidh · 2018-04-25T05:33:03Z

pkgs/development/tools/jira_cli/default.nix

+
+    # Tests rely on VCR cassettes being written during tests. R/O nix store prevents this.
+    doCheck = false;
+    #checkInputs = with pythonPackages; [ vcrpy mock hiro ];


nyarly · 2018-04-26T04:45:53Z

@FRidh is there existing documentation about the move into python-modules? I think it probably makes sense, but AFAICT the header of top-level/python-packages.nix and the Python section of the manual don't refer to it (yet.) If not, is it okay if I issue a separate PR to that effect?

nyarly · 2018-04-30T18:54:18Z

@FRidh How about now?

nyarly · 2018-05-13T01:00:34Z

@FRidh Merged master (with alternative version of the jira package) into this branch. How's it look now?

FRidh · 2018-05-13T07:10:48Z

How about now?

Oh, I missed that.

Please don't do any merges, but instead rebase.

FRidh · 2018-05-13T07:11:25Z

See also the contributing guidelines; one commit per logical change, which in that case would correspond to a commit per package.

nyarly · 2018-05-13T08:13:48Z

@FRidh I can't find contribution guidelines about git usage within nixpkgs. I have to admit to some frustration about the process around adding this expression. I'm willing to wrap up with a rebase, but I'd like to know that I'm done before I do that.

FRidh · 2018-05-13T08:18:11Z

@nyarly https://nixos.org/nixpkgs/manual/#idm140737315810544
mentions logical units and rebasing onto master. That one should not merge is not mentioned. Maybe it should be added?

I can understand the frustration, there are a lot of requirements and we're not always that quick with replying; there are just too many PR's and issues to handle by the community.

In any case, the changes themselves look good.

nyarly · 2018-05-13T08:41:22Z

@FRidh I appreciate your empathy. And I also understand that the number of PRs and issues outstrips the amount of time and attention available to review them. Thanks for all you're doing, and specifically for giving feedback to bring this home.

I've rebased and squashed this onto master. I hope it tells a clear narrative.

FRidh · 2018-05-13T08:46:18Z

Still needed is a commit per package. You can squash your commits, do a soft reset, and then create a commit for each.

nyarly · 2018-05-13T21:36:31Z

@FRidh This is what you're looking for?

nyarly · 2018-05-16T18:47:46Z

@FRidh Friendly reminder of this PR.

FRidh · 2018-05-17T06:53:17Z

Yep, thanks. In the future please also prepend the package set the packages are defined in.

FRidh added the 6.topic: python label Oct 26, 2017

matthewbauer previously requested changes Apr 21, 2018

View reviewed changes

nyarly requested a review from FRidh as a code owner April 22, 2018 03:00

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 22, 2018

FRidh requested changes Apr 23, 2018

View reviewed changes

FRidh requested changes Apr 25, 2018

View reviewed changes

FRidh reviewed Apr 25, 2018

View reviewed changes

nyarly force-pushed the jira_cli branch from cf69cad to f617a8d Compare May 13, 2018 08:38

hiro: init at 0.1.4

f252c72

nyarly added 2 commits May 13, 2018 14:20

keyrings.alt: init at 2.3

42d0ce8

jira_cli: init at 2.2

51f4a46

nyarly force-pushed the jira_cli branch from f617a8d to 51f4a46 Compare May 13, 2018 21:35

FRidh merged commit de55b71 into NixOS:master May 17, 2018

		@@ -2993,6 +2993,23 @@ in {

		};

		hiro = buildPythonPackage rec {

		@@ -6880,6 +6920,34 @@ in {

		jinja2_pluralize = callPackage ../development/python-modules/jinja2_pluralize { };

		jira = buildPythonPackage rec {

(jira-cli) init at 2.2 #30833

(jira-cli) init at 2.2 #30833

Conversation

nyarly commented Oct 26, 2017

FRidh commented Oct 26, 2017

nyarly commented Oct 26, 2017

nyarly commented Nov 22, 2017

FRidh commented Nov 25, 2017

matthewbauer left a comment

Choose a reason for hiding this comment

nyarly commented Apr 21, 2018

nyarly commented Apr 22, 2018

FRidh commented Apr 22, 2018

nyarly commented Apr 22, 2018

FRidh left a 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

nyarly commented Apr 23, 2018

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

nyarly commented Apr 26, 2018

nyarly commented Apr 30, 2018

nyarly commented May 13, 2018

FRidh commented May 13, 2018

FRidh commented May 13, 2018

nyarly commented May 13, 2018

FRidh commented May 13, 2018

nyarly commented May 13, 2018

FRidh commented May 13, 2018

nyarly commented May 13, 2018

nyarly commented May 16, 2018

FRidh commented May 17, 2018