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.

Sign up for GitHub

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

Jump to bottom

Add checkPhase to pythonPackages.astropy and several other related packages #65295

Merged
merged 7 commits into from Jul 27, 2019
Merged

Add checkPhase to pythonPackages.astropy and several other related packages #65295

merged 7 commits into from Jul 27, 2019

Conversation

smaret
Copy link
Member

@smaret smaret commented Jul 23, 2019 •
edited

Motivation for this change

Add a checkPhase in pythonPackages.astropy and several other related packages.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@smaret smaret requested a review from FRidh as a code owner July 23, 2019 09:55
@smaret
Copy link
Member Author

smaret commented Jul 23, 2019 •
edited

cc @costrouc @KentJames

@smaret
Copy link
Member Author

smaret commented Jul 23, 2019

@GrahamcOfBorg build python3Packages.astropy

@smaret smaret changed the title pythonPackages.astropy: add checkPhase. pythonPackages.astropy: add checkPhase Jul 23, 2019
@smaret smaret changed the title pythonPackages.astropy: add checkPhase Add checkPhase to pythonPackages.astropy and several other affiliated packages Jul 23, 2019
@smaret smaret changed the title Add checkPhase to pythonPackages.astropy and several other affiliated packages Add checkPhase to pythonPackages.astropy and several other related packages Jul 23, 2019
@smaret smaret changed the title Add checkPhase to pythonPackages.astropy and several other related packages [WIP] Add checkPhase to pythonPackages.astropy and several other related packages Jul 23, 2019
@costrouc
Copy link
Member

costrouc commented Jul 23, 2019 •
edited

Thank you for adding testing!

risicle
risicle reviewed Jul 23, 2019
View reviewed changes
pkgs/development/python-modules/astroquery/default.nix Outdated
src = fetchPypi {
inherit pname version;
sha256 = "0zw3xp2rfc6h2v569iqsyvzhfnzp7bfjb7jrj61is1hrqw1cqjrb";
};

# Fix tests using conftest.py from HEAD in the upstream GitHub
# repository.
patches = [ ./conftest.patch ];
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably better to use fetchpatch and extract the .patch from the actual PR (or just commit) upstream that actually fixed the issue. See other uses of fetchpatch - this is a common pattern.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what I did for other packages, but for this one several commits are needed. So rather than fetching multiple commits, I created a single patch from these.

@risicle
Copy link
Contributor

risicle commented Jul 23, 2019

Minor comments, but all works for me & nox-review passes on non-nixos linux x86_64.

Good work!

@smaret
Copy link
Member Author

smaret commented Jul 24, 2019

@GrahamcOfBorg build python3Packages.astroquery python3Packages.pyregion python3Packages.aplpy python3Packages.radio_beam python3Packages.spectral-cube python3Packages.astropy python3Packages.pytest-arraydiff

@smaret smaret changed the title [WIP] Add checkPhase to pythonPackages.astropy and several other related packages Add checkPhase to pythonPackages.astropy and several other related packages Jul 24, 2019
@FRidh FRidh merged commit 1354a2e into NixOS:master Jul 27, 2019
@smaret smaret deleted the astropy-checkphase branch July 27, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@risicle risicle risicle left review comments

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@smaret @costrouc @risicle @FRidh