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

Do the lint, docs and blacken sessions really need to be pinned to specific pythons? #531

Closed
FollowTheProcess opened this issue Dec 24, 2021 · 2 comments · Fixed by #532
Closed
Labels
ci Issues relating to nox's CI pipeline

Comments

@FollowTheProcess
Copy link
Collaborator

In our noxfile, the lint, doc, and blacken sessions are pinned to specific pythons using:

@nox.session(python="3.9")

As recently discovered (and quickly fixed! #529 Thanks @henryiii) this caused linting and docs building to quietly "pass" on GitHub actions (see here).

I can recall this happening once before too (however I can't seem to find it now).

The pinning of these sessions also causes a contributor to have to use --force-python if they don't have the specific version that the sessions are pinned to.

I think the best way to ensure a high quality contributing experience is for any contributor to be able to simply run nox and if everything passes, they can be confident that the CI for their contribution will pass. Implicitly skipping some of the sessions might get in the way of this goal.

If there is no specific reason these sessions are pinned I would suggest changing their session decorators to simply @nox.session to allow them to run under the default python (as the cover session is already).

If however there is a reason, I suggest we edit the CI to use the --error-on-missing-interpreters switch so that these sessions cannot silently "pass" without running.

Thoughts?

@FollowTheProcess FollowTheProcess added the ci Issues relating to nox's CI pipeline label Dec 24, 2021
@cjolowicz
Copy link
Collaborator

cjolowicz commented Dec 24, 2021

For the docs session, prioritizing ease of contributing over deterministic checks makes sense to me.

I'm less sure about the lint and blacken sessions. Flake8, mypy, isort, and black parse the Python AST. So it seems best to run them on a well-defined version of Python, and probably the latest stable version. I agree that --error-on-missing-interpreters is a good idea for CI in this case.

@henryiii
Copy link
Collaborator

Flake8, mypy, isort, and black all do try to be as consistent as possible. They might not support newer syntax from an older version, like 3.10+ only syntax when running on 3.9, but nox explicitly does not have anything that doesn't run on 3.6+ (soon 3.7+), so the runs should be exactly identical on 3.6+. I've never had an issue with avoiding pins for linters, and it is rather irritating to have the linting job fail and require you to install a specific Python version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Issues relating to nox's CI pipeline
Development

Successfully merging a pull request may close this issue.

3 participants