diff_cover: init at 1.0.2 #33715

dzabraev · 2018-01-10T19:04:42Z

Motivation for this change

new packages for diff_cover, pydocstyle, jinja2_pluralize

diff_cover depends on pydocstyle, jinja2_pluralize

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
Fits CONTRIBUTING.md.

teto · 2018-01-11T03:30:09Z

I've also packaged pydocstyle in #31180. Yours seem to enable tests but always including configparser might fail I believe.

FRidh · 2018-01-11T08:27:55Z

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

+  pname = "diff_cover";
+  version = "1.0.2";
+
+  LC_ALL = "en_US.UTF-8";


I think this is only needed for the tests

Yes it is. Without setting this variable tests will fail. Where should I put this variable definition?

Without this variable tests fail, if this variable will be visible only during tests execution,
runtime execution may fail too without LC_ALL.

The env var can be set in the preCheck hook. This env var won't be set during runtime anyway.

This package supply some executable files. If I wrap this executables with makeWrapper and set LC_ALL in wrapper?

FRidh · 2018-01-11T08:28:04Z

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

+
+  propagatedBuildInputs = [ jinja2 jinja2_pluralize pygments six inflect ];
+
+  buildInputs = [ mock nose coverage pycodestyle flake8 pyflakes pylint pydocstyle git ];


test requirements go into checkInputs

FRidh · 2018-01-11T08:28:16Z

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

+
+  LC_ALL = "en_US.UTF-8";
+
+  src = fetchFromGitHub {


any reason for not using PyPI?

Package from PyPi fails tests when build is doing in sandbox. I told about that to author of diff-cover. He fixed that bug.

Here this conversation about this bug.
Bachmann1234/diff_cover#72

Good work! If it is a patch on top of a release, then we fetch the patch with fetchpatch instead and add it to patches.

FRidh · 2018-01-11T08:28:33Z

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

+  src = fetchFromGitHub {
+    owner = "Bachmann1234";
+    repo = "diff-cover";
+    rev = "85c30959c8ed2aa3848f400095a2418f15bb7777";


you can refer to a tag here

If I refer to the last tag, tests will fail. Maybe it is better refer to this commit?

#33715 (comment)

FRidh · 2018-01-11T08:29:11Z

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

+  six, inflect, mock, nose, coverage, pycodestyle, flake8, pyflakes, git,
+  pylint, pydocstyle }:
+
+buildPythonPackage rec {


Does this tool need to access Python modules of the environment it is used in, or is it just an external tool?

( Maybe I didn't understand what you ask. I'm learning english ... :) )
This tool is not supposed to provide python modules for users.
It provide two execution files: diff-cover, diff-quality

This tool doesn't need access to environment where it is used in.

FRidh · 2018-01-11T08:29:32Z

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

+
+  propagatedBuildInputs = [ snowballstemmer configparser ];
+
+  buildInputs = [ pytest pytestpep8 mock pathlib ];


checkInputs

FRidh · 2018-01-11T08:29:57Z

pkgs/top-level/python-packages.nix

@@ -22660,6 +22660,13 @@ EOF
  voluptuous = callPackage ../development/python-modules/voluptuous { };

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


Attributes are roughly organized. Surely you can find a better place for these.

What is criteria of "better place" ?
I don't understand why this place is bad.

The attributes should be ordered by name.

Should be order by derivation name or by attribute name?
I still does not understand what you want because the attribute set doesn't have lexicographical order...

If i put this attribute near line 9493 (near jinja package) will it be ok?

dzabraev · 2018-01-17T18:47:14Z

@FRidh , why didnt you merge this request? I did all changes that you required.

FRidh · 2018-01-18T08:55:39Z

@dzabraev

Without an explicit mention I am not aware of the changes you made;
Lack of time.

FRidh · 2018-01-18T10:18:10Z

Please squash the commits so there is a commit per expression.

dzabraev · 2018-01-19T16:52:24Z

@FRidh I have done your requirements.

dzabraev requested a review from FRidh as a code owner January 10, 2018 19:04

dzabraev changed the title ~~Diff cover~~ diff_cover: init at 1.0.2 Jan 10, 2018

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jan 10, 2018

FRidh requested changes Jan 11, 2018

View reviewed changes

FRidh added the 6.topic: python label Jan 11, 2018

dzabraev force-pushed the diff_cover branch from 6f53972 to e82dd44 Compare January 19, 2018 16:24

dzabraev added 3 commits January 19, 2018 19:46

jinja2_pluralize: init at 0.3.0

87b2e17

pydocstyle: init at 0.3.2

a451f97

diff_cover: init at 1.0.2

16cc4ff

dzabraev force-pushed the diff_cover branch from e82dd44 to 16cc4ff Compare January 19, 2018 16:48

FRidh merged commit 018a210 into NixOS:master Jan 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

diff_cover: init at 1.0.2 #33715

diff_cover: init at 1.0.2 #33715

dzabraev commented Jan 10, 2018 •

edited

teto commented Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

FRidh Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

FRidh Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

FRidh Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

dzabraev Jan 12, 2018

FRidh Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

FRidh Jan 11, 2018

dzabraev Jan 11, 2018

dzabraev commented Jan 17, 2018 •

edited

FRidh commented Jan 18, 2018

FRidh commented Jan 18, 2018

dzabraev commented Jan 19, 2018


		propagatedBuildInputs = [ jinja2 jinja2_pluralize pygments six inflect ];

		buildInputs = [ mock nose coverage pycodestyle flake8 pyflakes pylint pydocstyle git ];


		propagatedBuildInputs = [ snowballstemmer configparser ];

		buildInputs = [ pytest pytestpep8 mock pathlib ];

diff_cover: init at 1.0.2 #33715

diff_cover: init at 1.0.2 #33715

Conversation

dzabraev commented Jan 10, 2018 • edited

Motivation for this change

Things done

teto commented Jan 11, 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

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

dzabraev commented Jan 17, 2018 • edited

FRidh commented Jan 18, 2018

FRidh commented Jan 18, 2018

dzabraev commented Jan 19, 2018

dzabraev commented Jan 10, 2018 •

edited

dzabraev commented Jan 17, 2018 •

edited