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.

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

pythonPackages.progressbar2: init at 3.12.0 #33920

Merged
merged 3 commits into from
Jan 17, 2018

Conversation

ashgillman
Copy link
Contributor

Motivation for this change

New python package, simple to use progress bar

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.

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you disable tests?

setup.py seems to download missing requirements:

Download error on https://pypi.python.org/simple/pytest-runner/: [Errno -2] Name or service not known -- Some packages may not be found!
Download error on https://pypi.python.org/simple/: [Errno -2] Name or service not known -- Some packages may not be found!

But no luck even if we use pytestrunner from nixpkgs, since pytest-runner>=2.8 is required. We will need to update it beforhand.

Maybe you didn't see the download errors if you don't have enabled sandboxing.

}:

buildPythonPackage (rec {
name = "${pname}-${version}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for name. Will be derived from pname and version.
Also, you can leave out ( and ).

sha256 = "16r21cpjvv0spf4mymgpy7hx6977iy11k44n2w9kipwg4lhwh02k";
};

buildInputs = [ pytest ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest belongs into checkInputs.

@ashgillman
Copy link
Contributor Author

Last commit should have dealt with all issues. I had a little trouble getting tests to run, and currently need to pull from GitHub, not PyPI (notified upstream, commented in expression).

@FRidh FRidh merged commit 5ab499d into NixOS:master Jan 17, 2018
@ashgillman ashgillman deleted the add-progressbar2 branch January 17, 2018 10:02
@dotlambda
Copy link
Member

@FRidh Shouldn't have been merged. I still get

Download error on https://pypi.python.org/simple/pytest-runner/: [Errno -2] Name or service not known -- Some packages may not be found!

We need to add pytestrunner as a dependency.

@FRidh
Copy link
Member

FRidh commented Jan 17, 2018

Right, I missed that.

It seems pytestrunner needs to be split out and updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants