-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
python3Packages.hydra-check: init at 1.1.1 #84917
Conversation
64165fa
to
63b4791
Compare
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.
Great to see this making its way into nixpkgs proper!
repo = "hydra-check"; | ||
rev = version; | ||
sha256 = "1dmsscsib8ckp496gsfqxmq8d35zs71n99xmziq9iprvy7n5clq2"; | ||
|
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.
Nit: extra whitespace here
flake8 hydracheck | ||
echo -e "\x1b[32m## run mypy\x1b[0m" | ||
mypy hydracheck | ||
''; |
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.
I would not do any of these checks inside the nix distribution: this will cause build failures when someone else goes to upgrade the version of black
or flake8
or mypy
, but these linting considerations are at the project src repo level, so they belong just in your CI for the project.
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.
i agree with black and flake8 but mypy seems to also check type hints. It might be useful to include it in case something changes with the next python version, no?
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.
I think that's fair. But mypy has also know to break some APIs, although I can't think of any in particular.
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.
or maybe it was that it became more strict. Either way, I've seen where a bump in mypy has caused failures
''; | ||
|
||
meta = with lib;{ | ||
homepage = https://github.com/nix-community/hydra-check; |
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.
homepage = https://github.com/nix-community/hydra-check; | |
homepage = "https://github.com/nix-community/hydra-check"; |
Quote for NixOS/rfcs#45
63b4791
to
b02211c
Compare
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.
LGTM
Result of nixpkgs-review pr 84917 1
Motivation for this change
Add hydra-check into upstream nixpkgs. closes nix-community/hydra-check#6
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)