python3Packages.lintrunner: init at 0.12.7#377405
python3Packages.lintrunner: init at 0.12.7#377405PrestonHager wants to merge 8 commits intoNixOS:masterfrom
Conversation
d7051df to
cc51b5f
Compare
There was a problem hiding this comment.
I think this tool is an application but not a library.
If so, please use buildPythonApplication and move it to pkgs/by-name/li/lintrunner instead.
There was a problem hiding this comment.
It is an application, I've used buildPythonApplication instead of buildPythonPackage. The typical way to install the application is via pip, should the package then be referenced/placed in the python top-level packages? This way it can be installed using python3Packages.lintrunner instead of the package name lintrunner, or is this bad practice?
There was a problem hiding this comment.
Could you add tests with pythonImportsCheck and pytestCheckHook?
https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#testing-python-packages-testing-python-packages
There was a problem hiding this comment.
Since it's an application, there's no imports to check with python. It's written in rust, should I run cargo test under the package to ensure tests? Not quite the same as testing the installed binary, but testing the application code functionality itself.
Added myself as a maintainer for future packages.
Based on https://github.com/suo/lintrunner upstream. Package is listed in PyPI already at https://pypi.org/project/lintrunner/. Built using buildPythonApplication with maturin (rust) build system.
cc51b5f to
b01f4fd
Compare
|
@natsukium I was able to make most of the changes you suggested, I have a couple of questions with regards to some changes. I left comments under each suggestion, but here they are below for completeness:
Thank you for the review and all the help! |
|
|
||
| python3Packages.buildPythonApplication rec { | ||
| pname = "lintrunner"; | ||
| version = "0.12.7"; |
There was a problem hiding this comment.
| version = "0.12.7"; | |
| version = "0.12.7-unstable-2025-07-03"; |
Since you aren't using the tagged release we should add the commit date here. Also please put a comment why the tagged release is not used. (And meta.changelog needs to be adjusted)
Based on https://github.com/suo/lintrunner upstream. Package is listed in PyPI already at https://pypi.org/project/lintrunner/.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.