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

leo-editor: 5.5 -> 5.6, add docutils and use pyqt56 #30031

Merged
merged 1 commit into from Feb 17, 2018
Merged

leo-editor: 5.5 -> 5.6, add docutils and use pyqt56 #30031

merged 1 commit into from Feb 17, 2018

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Oct 2, 2017

Motivation for this change

Update leo to new version, recently released.

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
    • Linux
  • 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.

@dtzWill dtzWill requested a review from FRidh as a code owner October 2, 2017 20:17
@dtzWill
Copy link
Member Author

dtzWill commented Oct 2, 2017

pyqt56 is introduced and used to fix themes, I tried it on a whim since after seeing it mentioned somewhere. It's our normal pyqt built against 5.6 instead of 5.9.

FRidh
FRidh requested changes Oct 2, 2017
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
@@ -257,6 +257,10 @@ in {
pythonPackages = self;
};

pyqt56 = pkgs.libsForQt56.callPackage ../development/python-modules/pyqt/5.x.nix {
Copy link
Member

Choose a reason for hiding this comment

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

Keep this private to leo-editor. We shouldn't have multiple versions of a package in python-packages.nix

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, sure thing. Sorry about that (pushed new commit with requested change).

@dtzWill
Copy link
Member Author

dtzWill commented Jan 3, 2018

rebased to latest master, ping!

FRidh
FRidh reviewed Jan 4, 2018
View reviewed changes
pkgs/applications/editors/leo-editor/default.nix Outdated
@@ -1,20 +1,20 @@
{ stdenv, python3Packages, fetchFromGitHub, makeWrapper, makeDesktopItem }:
{ stdenv, python3Packages, pyqt56, fetchFromGitHub, makeWrapper, makeDesktopItem }:

Copy link
Member

Choose a reason for hiding this comment

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

Overriding the Python package set here, replacing the pyqt5 attribute would be the preferred solution.

Copy link
Member

Choose a reason for hiding this comment

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

mitmproxy is a good example

@dtzWill
Copy link
Member Author

dtzWill commented Jan 7, 2018

Sorry for the delay--pushed new commit, quite a bit simpler!

Turns out there's already a pyqt56 in the python package set, so just using that instead.

@joachifm joachifm merged commit 67c5cb2 into NixOS:master Feb 17, 2018
@joachifm
Copy link
Contributor

I believe the review comments were addressed. Thank you

@joachifm
Copy link
Contributor

Ah, turns out pyqt56 ended up being removed at 964da1b. My bad.

@joachifm
Copy link
Contributor

I reverted this. Sorry for the inconvenience.

joachifm added a commit that referenced this pull request Feb 17, 2018
@joachifm
Revert "Merge pull request #30031 from dtzWill/update/leo-5.6"
05f5cdc 
This reverts commit 67c5cb2, reversing
changes made to 431c004.

I failed to notice that this depends on a package that had since
been removed. My bad :(
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Feb 18, 2018
@dtzWill
Revert "Revert "Merge pull request NixOS#30031 from dtzWill/update/le…
4139a6f 
…o-5.6""

This reverts commit 05f5cdc.
@dtzWill
Copy link
Member Author

dtzWill commented Feb 18, 2018

Erm, I'll open new one?

@dtzWill dtzWill mentioned this pull request Feb 18, 2018
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh requested changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 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
@dtzWill @joachifm @FRidh @GrahamcOfBorg