electrum: 3.2.4 -> 3.3.2 plus new dependencies. #54417

clefru · 2019-01-21T11:52:15Z

Motivation for this change

Upstream release. https://github.com/spesmilo/electrum/releases

Things done

clefru · 2019-01-21T12:11:32Z

dotlambda

Please read https://nixos.org/nixpkgs/manual/#python and https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes.

clefru · 2019-01-21T13:20:20Z

Please read https://nixos.org/nixpkgs/manual/#python and https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes.

Please be more specific.

dotlambda · 2019-01-21T13:30:56Z

The descriptions, attribute names and pnames need to be changed, meta.maintainers is missing.

dotlambda · 2019-01-21T13:32:07Z

pkgs/top-level/python-packages.nix

@@ -5193,6 +5195,8 @@ in {

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

+  aiorpcX = callPackage ../development/python-modules/aiorpcX { };


This should be sorted alphabetically - at least to some degree.

dotlambda

From https://github.com/romis2012/aiohttp-socks:

Python >= 3.5.3

From https://github.com/kyuupichan/aiorpcX:

Python (>= 3.6)

dotlambda · 2019-01-21T13:32:28Z

pkgs/applications/misc/electrum/default.nix

@@ -14,14 +14,17 @@ in

 python3Packages.buildPythonApplication rec {
  name = "electrum-${version}";


Suggested change

name = "electrum-${version}";

pname = "electrum";

This still needs to be changed.

Mic92 · 2019-01-21T13:52:14Z

Should this version be backported or is our current stable version fine (i.e. it does not break with other clients of the network)?

clefru · 2019-01-21T14:42:12Z

@dotlambda

From https://github.com/romis2012/aiohttp-socks:

Python >= 3.5.3

From https://github.com/kyuupichan/aiorpcX:

Python (>= 3.6)

Do you want disabled = pythonOlder "3.5.3"/"3.6" on the new aio* derivations?

The descriptions, attribute names and pnames need to be changed

Do you mean meta.description? That's there. What's "attribute names"? pnames need to be changed to what? They seem to fall in-line with what's already there. Do you mean the name/pname switch electrum? Why do you ask me to do that for electrum? It's already there, and breaks with the rule of "Don't ask for random unrelated clean-ups in PR".

meta.maintainers is missing.

Not going to add myself as maintainer. I apologize to add to the prior art of 1000 default.nix files under python-modules that has no maintainer.

Should this version be backported or is our current stable version fine (i.e. it does not break with other clients of the network)?

@Mic92 No backport required, imho. 3.2.4 addresses the security issues that 3.3.2 addresses as well.

dotlambda · 2019-01-21T14:55:37Z

Yes, disabled is needed.

Attribute names are the ones in python-packages.nix. Package names should adhere to the same name normalization rules (PEP 0503).

clefru · 2019-01-21T15:38:05Z

@dotlambda

Yes, disabled is needed.

I have added that.

Attribute names are the ones in python-packages.nix. Package names should adhere to the same name normalization rules (PEP 0503).

Wait, did you just take 3 message to tell me indirectly also via me reading a two-page PEP, that this should be aiohttp-socks instead of aiohttp_socks? I am used to a more welcoming and cooperating approach here on NixOS/nixpkgs. I like to apologize on your behalf for massively wasting my time. I hope that you are having a better day tomorrow.

@Mic92 From my side the review is complete and this is ready to be merged.

dotlambda · 2019-01-21T15:50:16Z

I like to apologize on your behalf for massively wasting my time. I hope that you are having a better day tomorrow.

Think about it the other way round: Would you have read https://nixos.org/nixpkgs/manual/#python before touching Python packages, I wouldn't have had to tell you to read it.

dotlambda · 2019-01-21T15:50:38Z

pkgs/applications/misc/electrum/default.nix

@@ -14,14 +14,17 @@ in

 python3Packages.buildPythonApplication rec {
  name = "electrum-${version}";


This still needs to be changed.

dotlambda · 2019-01-21T15:50:47Z

pkgs/development/python-modules/aiohttp-socks/default.nix

+{ lib, fetchPypi, buildPythonPackage, pythonOlder, aiohttp }:
+
+buildPythonPackage rec {
+  pname = "aiohttp_socks";


Suggested change

pname = "aiohttp_socks";

pname = "aiohttp-socks";

You will have to add pname = "aiohttp_socks" inside fetchPypi.

dotlambda · 2019-01-21T15:51:01Z

pkgs/development/python-modules/aiorpcX/default.nix

+{ lib, fetchPypi, buildPythonPackage, pythonOlder, attrs }:
+
+buildPythonPackage rec {
+  pname = "aiorpcX";


Suggested change

pname = "aiorpcX";

pname = "aiorpcx";

dotlambda · 2019-01-21T15:51:32Z

pkgs/top-level/python-packages.nix

@@ -5193,6 +5195,8 @@ in {

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

+  aiorpcX = callPackage ../development/python-modules/aiorpcX { };


Suggested change

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

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

dotlambda · 2019-01-21T15:53:40Z

pkgs/development/python-modules/aiohttp-socks/default.nix

+  disabled = pythonOlder "3.5.3";
+
+  meta = {
+    description = "SOCKS proxy connector for aiohttp.";


Suggested change

description = "SOCKS proxy connector for aiohttp.";

description = "SOCKS proxy connector for aiohttp";

See https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes.

dotlambda · 2019-01-21T15:53:53Z

pkgs/development/python-modules/aiorpcX/default.nix

+  propagatedBuildInputs = [ attrs ];
+
+  meta = {
+    description = "Transport, protocol and framing-independent async RPC client and server implementation.";


Suggested change

description = "Transport, protocol and framing-independent async RPC client and server implementation.";

description = "Transport, protocol and framing-independent async RPC client and server implementation";

clefru · 2019-01-21T16:01:29Z

I like to apologize on your behalf for massively wasting my time. I hope that you are having a better day tomorrow.

Think about it the other way round: Would you have read https://nixos.org/nixpkgs/manual/#python before touching Python packages, I wouldn't have had to tell you to read it.

That as helpful as "Read the library of congress", when it concerns such a small change.

Thanks your suggested changes. Would you have been more clear on what was to change, I would have had the time to pick them up. Unfortunately, this PR has exceed the time that I had allocated for it.

No, in all seriousness, you need to stop waste people's time with obscure guess-my-mind nitpicks. Closing.

dotlambda · 2019-01-21T16:06:57Z

You do have a point regarding the Python packaging guide. That's too long if your only goal is to update some package. Sorry about that. Must have been my prejudice against anything Bitcoin-related.

However, one could at least have expected you to read https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#submitting-changes since you ticked that box.

clefru requested a review from FRidh as a code owner January 21, 2019 11:52

GrahamcOfBorg added the 6.topic: python label Jan 21, 2019

clefru force-pushed the electrum_bump branch from 18888a5 to 402c378 Compare January 21, 2019 12:10

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jan 21, 2019

dotlambda requested changes Jan 21, 2019

View reviewed changes

dotlambda reviewed Jan 21, 2019

View reviewed changes

dotlambda requested changes Jan 21, 2019

View reviewed changes

clefru force-pushed the electrum_bump branch 3 times, most recently from 953f6f9 to 25b468c Compare January 21, 2019 15:29

clefru added 3 commits January 21, 2019 16:36

pythonPackages.aiohttp-socks: init at 0.2.2

0bbe1e2

pythonPackages.aiorpcX: init at 0.10.2

35857b7

electrum: 3.2.4 -> 3.3.2

a7a8433

clefru force-pushed the electrum_bump branch from 25b468c to a7a8433 Compare January 21, 2019 15:37

dotlambda requested changes Jan 21, 2019

View reviewed changes

clefru closed this Jan 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

electrum: 3.2.4 -> 3.3.2 plus new dependencies. #54417

electrum: 3.2.4 -> 3.3.2 plus new dependencies. #54417

clefru commented Jan 21, 2019

clefru commented Jan 21, 2019

dotlambda left a comment

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

dotlambda Jan 21, 2019

dotlambda left a comment

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019

Mic92 commented Jan 21, 2019 •

edited

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019 •

edited

dotlambda Jan 21, 2019

dotlambda Jan 21, 2019

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

		@@ -5193,6 +5195,8 @@ in {

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

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

		@@ -14,14 +14,17 @@ in

		python3Packages.buildPythonApplication rec {
		name = "electrum-${version}";

	description = "SOCKS proxy connector for aiohttp.";
	description = "SOCKS proxy connector for aiohttp";

	description = "Transport, protocol and framing-independent async RPC client and server implementation.";
	description = "Transport, protocol and framing-independent async RPC client and server implementation";

electrum: 3.2.4 -> 3.3.2 plus new dependencies. #54417

electrum: 3.2.4 -> 3.3.2 plus new dependencies. #54417

Conversation

clefru commented Jan 21, 2019

Motivation for this change

Things done

clefru commented Jan 21, 2019

dotlambda left a comment

Choose a reason for hiding this comment

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

Choose a reason for hiding this comment

dotlambda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Jan 21, 2019 • edited

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

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

dotlambda Jan 21, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clefru commented Jan 21, 2019

dotlambda commented Jan 21, 2019

Mic92 commented Jan 21, 2019 •

edited

dotlambda Jan 21, 2019 •

edited