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

Nikola CMS: init at 7.8.3 #22147

Closed
wants to merge 12 commits into from
Closed

Nikola CMS: init at 7.8.3 #22147

wants to merge 12 commits into from

Conversation

leenaars
Copy link
Contributor

Motivation for this change

Nikola is quite a popular and advanced python-based CMS, similar to Lektor.
It required a lot of updates to dependencies, and some new python dependencies, all of which are included.

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

@Mic92 Mic92 changed the title Nikola CMS: init -> 7.8.3 Nikola CMS: init at 7.8.3 Jan 25, 2017
@globin
Copy link
Member

globin commented Jan 25, 2017

Could you move the dependencies to seperate files in pkgs/development/python-modules/ and just call them from python-packages.nix. This will eventually happen with all the dependencies, the file is simply getting to large and this will make it easier to handle (git log, merge conflicts, etc.)

fpletz
fpletz reviewed Jan 25, 2017
View reviewed changes
pkgs/top-level/python-packages.nix
meta = {
description = "A static content management system";
homepage = "https://www.getnikola.com/";
license = "MIT";
Copy link
Member

Choose a reason for hiding this comment

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

Please use licenses.mit here.

FRidh
FRidh requested changes Jan 30, 2017
View reviewed changes
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

Are you going to maintain these Python packages?

pkgs/top-level/python-packages.nix

propagatedBuildInputs = with self; [ cloudpickle pyinotify pytest ];

doCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

include a reason why you disable them

pkgs/top-level/python-packages.nix
sha256 = "0ghj3bsrc5bm30w97bn8nvf5jlabg06xs16g7284mimyr7q9z0j0";
};

propagatedBuildInputs = with self; [ cloudpickle pyinotify pytest ];
Copy link
Member

Choose a reason for hiding this comment

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

does it need pytest during runtime??

pkgs/top-level/python-packages.nix
name = "${pname}-${version}";
version = "4.0.3";

# ImportError: No module named tests
Copy link
Member

Choose a reason for hiding this comment

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

ok...and how come? Should the test runner be invoked differently, or are tests simply missing? Just stating there was an importerror is not sufficient.

pkgs/top-level/python-packages.nix
sha256 = "18lna1bgzw4l3kqdi3nr1l557rj3wddrcj81x0rf8f4p87xsg9fq";
};

doCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

why?

pkgs/top-level/python-packages.nix
src = pkgs.fetchurl {
url = "mirror://pypi/N/${pname}/${name}.tar.gz";
sha256 = "12kcnvwy9dr194riji6iird1zpdcv252d19lfvabr39rsi9658jl";
};
Copy link
Member

Choose a reason for hiding this comment

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

are tests working?

pkgs/top-level/python-packages.nix
version = "1.3";
name = "${pname}-${version}";

doCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

why?

pkgs/top-level/python-packages.nix
version = "1.0.10";
name = "${pname}-${version}";

doCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

why?

pkgs/top-level/python-packages.nix
name = "pygal-${version}";
pname = "pygal";
version = "2.3.1";
name = "${pname}-${version}";

doCheck = !isPyPy; # one check fails with pypy
Copy link
Member

Choose a reason for hiding this comment

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

if its only one test, and the test is broken, then surely we can just disable that one test?

@FRidh
Copy link
Member

FRidh commented May 16, 2017

status update?

@leenaars leenaars closed this Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz fpletz left review comments

@FRidh FRidh FRidh requested changes

Assignees
No one assigned
Labels
2.status: work-in-progress 6.topic: python 8.has: package (new) 8.has: package (update)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@leenaars @globin @FRidh @fpletz @pSub