-
Notifications
You must be signed in to change notification settings - Fork 4
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
Cookiecutter proto #7
Conversation
session.run("ruff", "check", "--fix", ".") | ||
run_shellcheck(session, mode="fmt") |
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.
suggestion: I found really anoying that in single session we have multiple checks that has to run in single virtualenv - in https://github.com/backend-developers-ltd/compute-horde-facilitator-sdk/blob/master/noxfile.py I've used tags to control which sessions to select in nox run and then you can run single session or all sessions under choosen tag
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.
can you explain why did you find it annoying?
I personally disliked the fact, that I had to manually analyze each project just to know which commands I need to run before its safe to create a PR that won't error out on CI stage.
I just want a single command that I will run and I will be decently sure that my PR will pass CI.
I don't want to guess whenever mypy is needed or not for example.
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.
don't remember exactly what was the nail... :) but I wanted to have clean separate virtualenvs for different linters and be able to select one linter and run it - so sessions be like commands and tags be like sets of commands
it might be, that one thing was breaking and it was annoying to have to wait for all other linters to find out that still something is not working
tags works the same as sessions right now - just you can split linters per session, have seprate small virtualenvs and run single linter if you like
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.
and you can define default tags
nox.options.tags = ["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.
most linters will have to run in the same env tho, since any linter that requires to analyze whenever your imports make sense or type hints are correct requires all 3rd party stuff installed
also installing&activating N venvs rather 1 will be more resource intensive, ot less
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.
it was that running some docker thingy was taking some time and I needed to run the linter after this and was unable - but still - you can setup single venv and reuse between sessions :) but still - it is a suggestion that I came up with when trying to work with our projects and got frustrated much and that is why I don't use nox just "raw" ruff... I will need still doing it - not using nox :P
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.
The tag solution sounds good on paper, but having separate sessions with separate venvs has decremental performance implications.
I just tested and current nox -s lint
takes:
nox -s lint 1,87s user 0,46s system 74% cpu 3,139 total
and after breaking it into 3 groups (we could do even more, but it would be even worse then)
nox -t lint 5,15s user 1,28s system 70% cpu 9,172 total
(this is with warm cache; i.e. second runs, as that will be most often used scenario).
So if we start using some tool which takes >1s (i.e. longer than venv setup) less do a separate session for it, but otherwise keep it in the same session. Otherwise, we would be spending more time on linting instead of less.
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.
To facilitate tags use in future I will add the "lint" and "format" tags and recommend calling nox -t lint
in the README, so we are at least half way there.
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.
if it runs 1s that we have nothing to discuss - but it was taking like 60s on gtihub actions and also it was not this fast locally - but maybe it was misconfigured
No description provided.