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

Tooling: Confusing difference in behavior between pre-commit run --all-files and tox -e run lint #119

Closed
Sachaa-Thanasius opened this issue Jun 15, 2024 · 0 comments · Fixed by #118

Comments

@Sachaa-Thanasius
Copy link
Contributor

Sachaa-Thanasius commented Jun 15, 2024

I've noticed that if I run tox run -e lint in a clone of this project, black will do a few cosmetic changes related to whitespace and overload ellipses, while invoking pre-commit run --all-files will remove those changes. I thought tox's lint testenv was supposed to be run before trying to commit, so it was surprising to see that I should actually avoid using in order to prevent diffs that are going to get cancelled out anyway by pre-commit's black run.

Would you agree that this is potentially confusing for new contributors (e.g. me) trying to configure their environment? If so, a few potential fixes come to mind:

  • Merging [pre-commit.ci] pre-commit autoupdate #115, so that the version of black that pre-commit is using stays up to date with the one tox is using.
  • Pinning black within the tox lint testenv so that it's the same version as the pre-commit one.
  • Removing the lint testenv entirely and just implying tox -e run pre-commit or pre-commit run is what potential contributors should use.

I don't know your exact workflow, whether pinning black in pre-commit and in tox is potentially problematic, etc., so I don't know what's best; just pointing it out so maybe it can be simplified to one command for running linting/testing tools, slightly easing the contribution process.

Attached below is the output from both commands to demonstrate the behavior, using pre-commit 3.7.1 and tox 4.15.1:

PS C:\Users\name\Projects\personal\rfc3986> tox run -e lint
lint: commands[0]> black -l 79 -t py37 src/rfc3986 tests/
reformatted C:\Users\name\Projects\personal\rfc3986\src\rfc3986\compat.py
reformatted C:\Users\name\Projects\personal\rfc3986\src\rfc3986\exceptions.py
reformatted C:\Users\name\Projects\personal\rfc3986\src\rfc3986\uri.py
reformatted C:\Users\name\Projects\personal\rfc3986\src\rfc3986\iri.py
reformatted C:\Users\name\Projects\personal\rfc3986\src\rfc3986\_mixin.py
reformatted C:\Users\name\Projects\personal\rfc3986\tests\test_validators.py

All done! ✨ 🍰 ✨
6 files reformatted, 20 files left unchanged.
lint: commands[1]> flake8 src/rfc3986
src/rfc3986\parseresult.py:16:1: I100 Import statements are in the wrong order. 'from collections import namedtuple' should be before 'import typing'
lint: exit 1 (1.95 seconds) C:\Users\name\Projects\personal\rfc3986> flake8 src/rfc3986 pid=44572
  lint: FAIL code 1 (2.91=setup[0.08]+cmd[0.88,1.95] seconds)
  evaluation failed :( (3.09 seconds)
PS C:\Users\name\Projects\personal\rfc3986> git diff
diff --git a/src/rfc3986/_mixin.py b/src/rfc3986/_mixin.py
index f55d13f..4686a65 100644
--- a/src/rfc3986/_mixin.py
+++ b/src/rfc3986/_mixin.py
@@ -1,4 +1,5 @@
 """Module containing the implementation of the URIMixin class."""
+
 import warnings

 from . import exceptions as exc
diff --git a/src/rfc3986/compat.py b/src/rfc3986/compat.py
index d509d64..acddfb9 100644
--- a/src/rfc3986/compat.py
+++ b/src/rfc3986/compat.py
@@ -24,8 +24,7 @@ __all__ = (
 def to_str(  # noqa: D103
     b: t.Union[str, bytes, bytearray],
     encoding: str = "utf-8",
-) -> str:
-    ...
+) -> str: ...


 @t.overload
@@ -47,8 +46,7 @@ def to_str(
 def to_bytes(  # noqa: D103
     s: t.Union[str, bytes, bytearray],
     encoding: str = "utf-8",
-) -> bytes:
-    ...
+) -> bytes: ...


 @t.overload
diff --git a/src/rfc3986/exceptions.py b/src/rfc3986/exceptions.py
PS C:\Users\name\Projects\personal\rfc3986> git diff
diff --git a/src/rfc3986/_mixin.py b/src/rfc3986/_mixin.py
index f55d13f..4686a65 100644
--- a/src/rfc3986/_mixin.py
+++ b/src/rfc3986/_mixin.py
@@ -1,4 +1,5 @@
 """Module containing the implementation of the URIMixin class."""
+
 import warnings

 from . import exceptions as exc
diff --git a/src/rfc3986/compat.py b/src/rfc3986/compat.py
index d509d64..acddfb9 100644
--- a/src/rfc3986/compat.py
+++ b/src/rfc3986/compat.py
@@ -24,8 +24,7 @@ __all__ = (
 def to_str(  # noqa: D103
     b: t.Union[str, bytes, bytearray],
     encoding: str = "utf-8",
-) -> str:
-    ...
+) -> str: ...


 @t.overload
@@ -47,8 +46,7 @@ def to_str(
 def to_bytes(  # noqa: D103
     s: t.Union[str, bytes, bytearray],
     encoding: str = "utf-8",
-) -> bytes:
-    ...
+) -> bytes: ...


 @t.overload
diff --git a/src/rfc3986/exceptions.py b/src/rfc3986/exceptions.py
index d513ddc..45c9c8f 100644
--- a/src/rfc3986/exceptions.py
+++ b/src/rfc3986/exceptions.py
@@ -1,4 +1,5 @@
 """Exceptions module for rfc3986."""
+
 from . import compat


diff --git a/src/rfc3986/iri.py b/src/rfc3986/iri.py
index 6578bc0..a275b81 100644
--- a/src/rfc3986/iri.py
+++ b/src/rfc3986/iri.py
@@ -1,4 +1,5 @@
 """Module containing the implementation of the IRIReference class."""
+
 # Copyright (c) 2014 Rackspace
 # Copyright (c) 2015 Ian Stapleton Cordasco
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/src/rfc3986/uri.py b/src/rfc3986/uri.py
index 6747447..29a9f54 100644
--- a/src/rfc3986/uri.py
+++ b/src/rfc3986/uri.py
@@ -1,4 +1,5 @@
 """Module containing the implementation of the URIReference class."""
+
 # Copyright (c) 2014 Rackspace
 # Copyright (c) 2015 Ian Stapleton Cordasco
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/tests/test_validators.py b/tests/test_validators.py
index 0ec7449..2bab3a1 100644
--- a/tests/test_validators.py
+++ b/tests/test_validators.py
@@ -1,4 +1,5 @@
 """Tests for the validators module."""
+
 import pytest

 import rfc3986
PS C:\Users\name\Projects\personal\rfc3986> pre-commit run --all-files
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
Reorder python imports...................................................Failed
- hook id: reorder-python-imports
- exit code: 1
- files were modified by this hook

Reordering imports in src/rfc3986/exceptions.py
Reordering imports in tests/test_validators.py
Reordering imports in src/rfc3986/uri.py
Reordering imports in src/rfc3986/_mixin.py
Reordering imports in src/rfc3986/iri.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted src\rfc3986\compat.py

All done! \u2728 \U0001f370 \u2728
1 file reformatted, 27 files left unchanged.

pyupgrade................................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

src/rfc3986/parseresult.py:16:1: I100 Import statements are in the wrong order. 'from collections import namedtuple' should be before 'import typing'

setup-cfg-fmt............................................................Passed
PS C:\Users\name\Projects\personal\rfc3986> git diff
PS C:\Users\name\Projects\personal\rfc3986> 
@Sachaa-Thanasius Sachaa-Thanasius changed the title Tooling: Different behavior in black when invoked via pre-commit run --all-files vs tox -e run lint Tooling: Different behavior between pre-commit run --all-files and tox -e run lint Jun 15, 2024
@Sachaa-Thanasius Sachaa-Thanasius changed the title Tooling: Different behavior between pre-commit run --all-files and tox -e run lint Tooling: Confusing difference in behavior between pre-commit run --all-files and tox -e run lint Jun 15, 2024
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 a pull request may close this issue.

1 participant