-
-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Posting: init at 1.10.0 #325971
base: master
Are you sure you want to change the base?
Posting: init at 1.10.0 #325971
Conversation
pkgs/development/python-modules/textual-autocomplete/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/python-modules/textual-autocomplete/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/python-modules/textual-autocomplete/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/python-modules/textual-autocomplete/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/python-modules/textual-autocomplete/default.nix
Outdated
Show resolved
Hide resolved
]; | ||
|
||
pythonImportsCheck = [ "textual_autocomplete" ]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you enable tests with pytestCheckHook
?
Make sure the tests are enabled using for example pytestCheckHook and, in the case of libraries, are passing for all interpreters. If certain tests fail they can be disabled individually. Try to avoid disabling the tests altogether. In any case, when you disable tests, leave a comment explaining why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither textual-autocomplete
nor posting
have tests implemented. Pytest returns an error when no test is present and this breaks the build process.
See: https://github.com/darrenburns/textual-autocomplete/tree/version-two/tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may add this instead then
doCheck = false; # no tests |
(refer pr in packaging request) |
9ecfa7a
to
16025c1
Compare
@natsukium this PR is ready. Can you take a look please? |
pkgs/by-name/po/posting/package.nix
Outdated
fetchFromGitHub, | ||
}: | ||
|
||
python3.pkgs.buildPythonApplication rec { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like python3.pkgs
causes issue with splicing, and the fix is to use python3Packages
instead (and below)
pkgs/by-name/po/posting/package.nix
Outdated
src = fetchFromGitHub { | ||
owner = "darrenburns"; | ||
repo = "posting"; | ||
rev = version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rev = version; | |
rev = f"refs/tags/${version}"; |
pythonRelaxDeps = true; | ||
|
||
pythonImportsCheck = [ "posting" ]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nativeCheckInputs = [ pytestCheckHook ]; |
src = fetchFromGitHub { | ||
owner = "darrenburns"; | ||
repo = "textual-autocomplete"; | ||
rev = "bbacfa91bfd9ff006dab8930a8a3fe4ba46853ab"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mayybe add a comment explaining that this commit target the version-two
branch
]; | ||
|
||
pythonImportsCheck = [ "textual_autocomplete" ]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may add this instead then
doCheck = false; # no tests |
@Sigmanificient thanks for your feedback. I took into account your suggestions and noticed that tests were added upstream. I tried adding them here but couldn't make them pass. I would appreciate if someone could give it a look. Otherwise should we just disable them? (Nice seeing a fellow Rennais here, did all my studies in Rennes and only recently left :) ) |
Description of changes
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.