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

chore: run pre-commit on all files #1474

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dimaqq
Copy link
Contributor

@dimaqq dimaqq commented Nov 28, 2024

Apparently pre-commit was not run on a few occasions and here's the diff.

I'm not sure about dangling whitespace in the pebble test.

@dimaqq
Copy link
Contributor Author

dimaqq commented Nov 28, 2024

P.S. pre-commit also reports a few unused # noqa: ... that it doesn't fix automatically.

@dimaqq
Copy link
Contributor Author

dimaqq commented Nov 28, 2024

Indeed the trailing spaces in test were significant.
This needs a work-around, some ruff pragma to keep those, or a modification to the test to ignore whitespace in comparison maybe?

@james-garner-canonical
Copy link
Contributor

Definitely seems problematic that running pre-commit with our config can break our tests

@dimaqq dimaqq marked this pull request as draft November 28, 2024 06:18
@dimaqq dimaqq force-pushed the chore-run-pre-commit branch from 87b9e82 to f0ff476 Compare November 28, 2024 06:28
@dimaqq dimaqq force-pushed the chore-run-pre-commit branch from f0ff476 to ea6f6cf Compare November 28, 2024 06:39
@dimaqq
Copy link
Contributor Author

dimaqq commented Nov 28, 2024

Apparently trialing whitespace fixer doesn't understand Python multiline string literals.
Arguably, dangling whitespace in those is not great, but let's keep it as is for now.
I've exempted that one file.

Re: RUF039, I think we should probably go through the half dozen instances where ruff insists on raw strings and update tests, wdyt?

@@ -1810,7 +1810,7 @@ def test_breakpoint_good_names(self, request: pytest.FixtureRequest, name: str):
'-',
'...foo',
'foo.bar',
'bar--' 'FOO',
'bar--FOO',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this was a bug, perhaps 'bar--', 'FOO' was meant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, looking at the original commit. Nice find!

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 this pull request may close these issues.

3 participants