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

python2Packages.plyplus: init at 0.7.5 #46845

Merged
merged 1 commit into from Nov 12, 2018
Merged

python2Packages.plyplus: init at 0.7.5 #46845

merged 1 commit into from Nov 12, 2018

Conversation

Twey
Copy link
Contributor

@Twey Twey commented Sep 18, 2018

Motivation for this change
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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@Twey Twey requested a review from FRidh as a code owner September 18, 2018 17:23
FRidh
FRidh previously requested changes Sep 18, 2018
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
@@ -8766,6 +8766,8 @@ in {

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

PlyPlus = callPackage ../development/python-modules/PlyPlus { };
Copy link
Member

Choose a reason for hiding this comment

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

lowercase attribute name

Copy link
Contributor Author

@Twey Twey Sep 19, 2018 •
edited

Choose a reason for hiding this comment

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

Do we not want to match the PyPI name, as with PyChromecast, Pmw, PyWebDAV, APScheduler, CommonMark, …? I find it much less confusing if I can just drop the name of a Python package into my derivation without having to look up what name transformations have been done on it (plyplus? plyPlus? ply-plus?).

Copy link
Member

Choose a reason for hiding this comment

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

No

@Twey
Copy link
Contributor Author

Twey commented Oct 1, 2018

Is this okay?

@Twey
Copy link
Contributor Author

Twey commented Oct 11, 2018

Hello?

@Twey Twey changed the title python2Packages.PlyPlus: init at 0.7.5 python2Packages.plyplus: init at 0.7.5 Oct 11, 2018
@FRidh
Copy link
Member

FRidh commented Oct 11, 2018

folder name should also follow the normalized form, so lowercase.

@Twey
Copy link
Contributor Author

Twey commented Oct 12, 2018

Done!
Is this the correct normalized form? Do we normalize to plyplus, plyPlus, ply-plus, or something else? Is this documented somewhere?

@Twey
Copy link
Contributor Author

Twey commented Nov 6, 2018

Hello?

@Twey
Copy link
Contributor Author

Twey commented Nov 7, 2018

Rebased to modern master.

@samueldr samueldr dismissed FRidh’s stale review November 12, 2018 03:24

Change has been made

@samueldr
Copy link
Member

Built fine locally, can't see anything wrong, and all asked-for changes were made.

Let's see how it copes on other platforms, as a formality.

@GrahamcOfBorg build python2Packages.plyplus

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: python2Packages.plyplus

Partial log (click to expand)

test_operators (plyplus.test.test_selectors.TestSelectors) ... ok
test_regexp_param (plyplus.test.test_selectors.TestSelectors) ... ok
test_tree_param (plyplus.test.test_selectors.TestSelectors) ... ok
test_yield (plyplus.test.test_selectors.TestSelectors) ... ok

----------------------------------------------------------------------
Ran 38 tests in 34.097s

OK
/nix/store/h8ybxafy31sdaq01nxdc7q0fk2szb8lq-python2.7-PlyPlus-0.7.5

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: python2Packages.plyplus

Partial log (click to expand)

test_deepcopy (plyplus.test.test_trees.TestSTrees) ... ok
test_parents (plyplus.test.test_trees.TestSTrees) ... ok
test_pickle (plyplus.test.test_trees.TestSTrees) ... ok
test_pickle_with_parents (plyplus.test.test_trees.TestSTrees) ... ok

----------------------------------------------------------------------
Ran 38 tests in 30.133s

OK
/nix/store/0lfxzhyjya62szzxk0zidyvi4jv9bv63-python2.7-PlyPlus-0.7.5

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: python2Packages.plyplus

Partial log (click to expand)

test_operators (plyplus.test.test_selectors.TestSelectors) ... ok
test_regexp_param (plyplus.test.test_selectors.TestSelectors) ... ok
test_tree_param (plyplus.test.test_selectors.TestSelectors) ... ok
test_yield (plyplus.test.test_selectors.TestSelectors) ... ok

----------------------------------------------------------------------
Ran 38 tests in 74.885s

OK
/nix/store/00r8l9ls7vy7qcsba24avna78qsb9gyg-python2.7-PlyPlus-0.7.5

@samueldr samueldr merged commit d746c53 into NixOS:master Nov 12, 2018
@Twey
Copy link
Contributor Author

Twey commented Nov 21, 2018

Thank you @samueldr!

@Twey Twey deleted the plyplus branch November 21, 2018 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh left review comments

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@Twey @FRidh @samueldr @GrahamcOfBorg