Split tox lint env into three envs, all safe #1870

EliahKagan · 2024-03-13T16:07:56Z

See 4c034db for details on the main changes.

This adds tox environments for both linting and format checking with Ruff, specifying Ruff as a dependency and calling it in such a way that it does not mutate files in the working tree, since one expects tox not to do that. This still has tox use pre-commit to run other hooks (which do not modify files).

Since none of these modify files, they all can and should run automatically when tox is run with no arguments, so they are added to the env_list (which the old lint env had been removed from in #1868, pending a change like this).

With this approach taken, there is no clear use for the already-unused manual hook stage in the near future anymore, and it's easy to bring back if needed, so I've also removed that, including in the CI linting workflow.

After adding the tox envs for Ruff, I tweaked them so they would show colored output, so then I went ahead and made a similar change for mypy when run with tox. I have included that here as well, even though it is not related to Ruff. But the changes here do not cause mypy to show colored output when run in the pythonpackage.yml workflow, because I ended up doing that in #1859 to help with the work there (alongside other mypy configuration improvements).

This is no longer used. No pre-commit hook specifies it anymore in `stages`, since 517f83a (gitpython-developers#1865). See b059cd5 (gitpython-developers#1868) for context. In the lint.yml GitHub Actions workflow, this removes the extra_args key altogether, because all that would remain there is --all-files, which is already the default for that action, when the extra_args key is absent.

It makes sense for ruff linting and ruff autoformatting to be easily runnable individually and to have their results be shown separately. Splitting them out in tox.ini also makes it so tox can do the other tests corresponding to those in lint.yml on CI in an environment that requries no custom behavior from pre-commit other than skipping the ruff checks. The ruff linting ("ruff") and ruff format checking ("format") tox environments specify ruff as a dependency and call it directly rather than through pre-commit, invoking it in such a way that it does not attempt to modify any files in the working tree. See b059cd5 (gitpython-developers#1868) for context. All three of these tox envs that "lint" has been split into are listed in the env_list and thus run automatically when tox is run with no arguments, since no tox envs unexpectedly (or at all) modify files in the working tree anymore. One limitation of the current approach is that new pre-commit hooks configured in .pre-commit-config.yml will automatically be run as part of the "misc" tox environment, which means that new unexpected mutating operatons could be added if the impact on tox is not considered. The benefit of having it work this way is that most hooks that are likely to be added to GitPython would not modify files and would be wanted as part of "misc". But this may benefit from further refinement.

In most other situations it is typically already colorized. This also includes comments about how to override this to suppress color.

On Unix-like systems, this approach is currently only sufficient if the TERM environment variable is set. In practice this means that if tox is run on CI, color will not be shown. Because GitPython does not itself use tox on CI, and other (e.g. downstream) projects may not want color on CI or in other situations where this would be insufficient, the further step of defining TERM as a workaround is deliberately omitted. See aeacb0 in gitpython-developers#1859 for what adding TERM would look like. This does not make any changes to how mypy runs on CI because that change is already included there.

Byron

Thanks a lot for this update - it's super exciting to see ruff (and Rust) work in GitPython :).

EliahKagan added 4 commits March 13, 2024 11:54

Colorize ruff output when run through tox

dcbd5db

In most other situations it is typically already colorized. This also includes comments about how to override this to suppress color.

EliahKagan marked this pull request as ready for review March 13, 2024 16:09

Byron approved these changes Mar 14, 2024

View reviewed changes

Byron merged commit 7734921 into gitpython-developers:main Mar 14, 2024
26 checks passed

EliahKagan deleted the tox-ruff branch March 14, 2024 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split tox lint env into three envs, all safe #1870

Split tox lint env into three envs, all safe #1870

EliahKagan commented Mar 13, 2024 •

edited

Loading

Byron left a comment

Split tox lint env into three envs, all safe #1870

Split tox lint env into three envs, all safe #1870

Conversation

EliahKagan commented Mar 13, 2024 • edited Loading

Byron left a comment

Choose a reason for hiding this comment

EliahKagan commented Mar 13, 2024 •

edited

Loading