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

Add the missing pyjson5 module #72974

Merged
merged 3 commits into from Nov 11, 2019
Merged

Add the missing pyjson5 module #72974

merged 3 commits into from Nov 11, 2019

Conversation

@isgy
Copy link
Member

isgy commented Nov 7, 2019

Motivation for this change

pyjson5 is needed by jupyterlab_server #72965

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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.
Notify maintainers

cc @FRidh @jonringer

@isgy isgy requested review from FRidh and jonringer as code owners Nov 7, 2019
@ofborg ofborg bot added the 6.topic: python label Nov 7, 2019
@FRidh

This comment has been minimized.

Copy link
Member

FRidh commented Nov 7, 2019

Please check the contributing guidelines for how to name commits. Merges should be avoided so please rebase instead.

Copy link
Member

veprbl left a comment

Commit name should read "pythonPackages.pyjson5: init at 0.8.5"

Copy link
Contributor

jonringer left a comment

the commit history should look like:

maintainers: add isgy
python3Packages.pyjson5: init at 0.8.5
@alexvorobiev

This comment has been minimized.

Copy link
Contributor

alexvorobiev commented Nov 7, 2019

Thanks for the fix! Should the derivation for jupyterlab_server be updated as well to add the dependency on pyjson5?

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 7, 2019

that is what the PR states :)
Personally, i would do that here, but if the PR wants to be broken into 2, then thats also okay

@isgy

This comment has been minimized.

Copy link
Member Author

isgy commented Nov 7, 2019

@jonringer @veprbl I'm not sure why the tests are failing (since I haven't touched any of the files in that list)

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 7, 2019

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 7, 2019

The error wasn't even from your package, that was weird

@ofborg ofborg bot requested a review from costrouc Nov 10, 2019
@FRidh

This comment has been minimized.

Copy link
Member

FRidh commented Nov 10, 2019

Always rebase, do not merge master.

@isgy

This comment has been minimized.

Copy link
Member Author

isgy commented Nov 10, 2019

sorry, I know.. fixing

@isgy

This comment has been minimized.

Copy link
Member Author

isgy commented Nov 10, 2019

@cdepillabout cdepillabout removed their request for review Nov 10, 2019
@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 11, 2019

please merge the two maintainer commits..

git rebase -i HEAD~4
# move the bottom (maintainer) line to below the first maintainer entry
# change pick to squash
# save
git push <fork> <branch> --force
maintainers: fixed typo
@isgy

This comment has been minimized.

Copy link
Member Author

isgy commented Nov 11, 2019

@jonringer @FRidh sorry.. good idea :)

Copy link
Contributor

jonringer left a comment

nix-review passes on NixOS
diff LGTM
commits LGTM

failures are broken on master

[6 built (2 failed), 3 copied (9.5 MiB), 9.2 MiB DL]
error: build of '/nix/store/5981hvalbm7s5kgz1sm4wwcd6c5k7pan-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/72974
2 package failed to build:
python37Packages.jupyterlab python38Packages.jupyterlab

5 package were build:
python27Packages.pyjson5 python37Packages.jupyterlab_server python37Packages.pyjson5 python38Packages.jupyterlab_server python38Packages.pyjson5
@jonringer jonringer merged commit da006ed into NixOS:master Nov 11, 2019
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.