From f78841870cea95926a95ccaae000bd51769ba412 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Mar 2024 10:46:53 -0400 Subject: [PATCH 1/4] Clean up mention of manual hook stage This is no longer used. No pre-commit hook specifies it anymore in `stages`, since 517f83a (#1865). See b059cd5 (#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. --- .github/workflows/lint.yml | 2 -- tox.ini | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a32fb6c4e..a0e81a993 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,5 +14,3 @@ jobs: python-version: "3.x" - uses: pre-commit/action@v3.0.1 - with: - extra_args: --all-files --hook-stage manual diff --git a/tox.ini b/tox.ini index 6e02e5aee..31ac9382d 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ commands = pytest --color=yes {posargs} [testenv:lint] description = Lint via pre-commit base_python = py{39,310,311,312,38,37} -commands = pre-commit run --all-files --hook-stage manual +commands = pre-commit run --all-files [testenv:mypy] description = Typecheck with mypy From 4c034db60e05f73104d265ec53099a8b00522269 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Mar 2024 11:27:07 -0400 Subject: [PATCH 2/4] Split tox "lint" env into three envs, all safe 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 (#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. --- tox.ini | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index 31ac9382d..1fc7e2415 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] requires = tox>=4 -env_list = py{37,38,39,310,311,312}, mypy, html +env_list = py{37,38,39,310,311,312}, ruff, format, mypy, html, misc [testenv] description = Run unit tests @@ -9,10 +9,17 @@ extras = test pass_env = SSH_* commands = pytest --color=yes {posargs} -[testenv:lint] -description = Lint via pre-commit +[testenv:ruff] +description = Lint with Ruff base_python = py{39,310,311,312,38,37} -commands = pre-commit run --all-files +deps = ruff +commands = ruff check . + +[testenv:format] +description = Check formatting with Ruff +base_python = py{39,310,311,312,38,37} +deps = ruff +commands = ruff format --check . [testenv:mypy] description = Typecheck with mypy @@ -28,3 +35,10 @@ allowlist_externals = make commands = make BUILDDIR={env_tmp_dir}/doc/build -C doc clean make BUILDDIR={env_tmp_dir}/doc/build -C doc html + +[testenv:misc] +description = Run other checks via pre-commit +base_python = py{39,310,311,312,38,37} +set_env = + SKIP = ruff-format,ruff +commands = pre-commit run --all-files From dcbd5dbc38d95dfd31dd5897cb7511a7fda0cce3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Mar 2024 11:38:44 -0400 Subject: [PATCH 3/4] Colorize ruff output when run through tox In most other situations it is typically already colorized. This also includes comments about how to override this to suppress color. --- tox.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tox.ini b/tox.ini index 1fc7e2415..48c9ed2f9 100644 --- a/tox.ini +++ b/tox.ini @@ -13,12 +13,16 @@ commands = pytest --color=yes {posargs} description = Lint with Ruff base_python = py{39,310,311,312,38,37} deps = ruff +set_env = + CLICOLOR_FORCE = 1 # Set NO_COLOR to override this. commands = ruff check . [testenv:format] description = Check formatting with Ruff base_python = py{39,310,311,312,38,37} deps = ruff +set_env = + CLICOLOR_FORCE = 1 # Set NO_COLOR to override this. commands = ruff format --check . [testenv:mypy] From 89e519e05ca3342c3747b19901b8bb6c1d2b7f58 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Mar 2024 11:47:50 -0400 Subject: [PATCH 4/4] Colorize mypy output when run through tox 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 #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. --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index 48c9ed2f9..d6c23bf60 100644 --- a/tox.ini +++ b/tox.ini @@ -28,6 +28,8 @@ commands = ruff format --check . [testenv:mypy] description = Typecheck with mypy base_python = py{39,310,311,312,38,37} +set_env = + MYPY_FORCE_COLOR = 1 commands = mypy -p git ignore_outcome = true