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.

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

Remove venv, migrate checks to flake check #7

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Conversation

henrirosten
Copy link
Collaborator

@henrirosten henrirosten commented Nov 28, 2023

  • Remove python venv usage and the unnecessary pip install run whenever the shell is entered. With this change, we can remove all Makefile targets and scripts related to managing that. This change is similar to the change done in: bring in python devshell with Nix sbomnix#94 (comment)
  • Moves the following checks from Makefile targets to nix flake check: black, pycodestyle, pylint
  • Use dep5 to bulk license set of files to remove the .license files
  • Smaller cleanups and VERSION update

Signed-off-by: Henri Rosten <[email protected]>
Signed-off-by: Henri Rosten <[email protected]>
Signed-off-by: Henri Rosten <[email protected]>
Signed-off-by: Henri Rosten <[email protected]>
@henrirosten henrirosten changed the title Remove venv Remove venv, migrate checks to flake check Nov 28, 2023
@henrirosten henrirosten marked this pull request as ready for review November 28, 2023 06:01
@henrirosten henrirosten requested review from brianmcgillion, brianmcgee and a team November 28, 2023 06:02
Copy link

@brianmcgillion brianmcgillion left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +22 to +39
# pycodestyle
pycodestyle =
pkgs.runCommandLocal "pycodestyle" {
nativeBuildInputs = [pkgs.python3.pkgs.pycodestyle];
} ''
cd ${../.}
pycodestyle --max-line-length 90 $(find . -name "*.py" ! -path "*venv*" ! -path "*eggs*")
touch $out
'';
# pylint
pylint =
pkgs.runCommandLocal "pylint" {
nativeBuildInputs = with pkgs.python3Packages; [colorlog gitpython pandas pylint pytest tabulate];
} ''
cd ${../.}
pylint --disable duplicate-code -rn $(find . -name "*.py" ! -path "*venv*" ! -path "*eggs*")
touch $out
'';
Copy link
Contributor

@brianmcgee brianmcgee Nov 28, 2023

Choose a reason for hiding this comment

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

We might be able to run these with treefmt. I think it can be configured to run arbitrary commands. Failing that, I might have a look at adding support directly for pycodestyle and pylint.

Copy link
Collaborator Author

@henrirosten henrirosten Nov 28, 2023

Choose a reason for hiding this comment

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

@brianmcgee : thanks for you comment. The changes in this PR are based on Florian's changes in: https://github.com/tiiuae/sbomnix/pull/94/files#diff-49f7a3ba809188883d2a60aef5bcd93529587f91ebb40004b7fc3f412feae79cR22-R38.

I propose we would merge this now, and then do another round of changes (to other repositories too) to move them to treefmt if it turns out it's easily doable.

Copy link
Contributor

@brianmcgee brianmcgee Nov 28, 2023

Choose a reason for hiding this comment

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

100%, I was suggesting it as a follow up, sorry for not making that clearer

@henrirosten henrirosten merged commit abed9e5 into main Nov 28, 2023
1 check passed
@henrirosten henrirosten deleted the remove-venv branch December 1, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants