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

Switch linting to ruff, use f-strings, fix lint errors and possible bugs #3126

Merged
merged 13 commits into from
Aug 15, 2023

Conversation

akx
Copy link
Contributor

@akx akx commented May 11, 2023

This PR is a bit on the big side, but it should be pretty straightforward to review commit by commit. A net line reduction
too!

It:

  • replaces .format and % string formatting with the more succinct and performant f-strings wherever possible (this was half powered by flynt, half manual work)
  • replaces the flake8 linter with the much faster, more capable ruff linter
  • ... and then starts to fix up various issues and possible bugs found by ruff. See each commit's message for details.

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

Thanks! I appreciate how methodical you were here.

milas added a commit to milas/docker-py that referenced this pull request Aug 15, 2023
@milas milas merged commit 8b9ad78 into docker:main Aug 15, 2023
8 of 10 checks passed
@milas
Copy link
Contributor

milas commented Aug 15, 2023

FYI the integration test failure was because the test was broken [before this PR] - it was missing assert so the statements were no-ops, the linter caught that but the asserts were incorrect. I fixed the status code in the merge after comparing to real engine behavior.

(Also apologies for noise to anyone watching this repo, it appears my Git pushes might have caused a bunch of notification emails 😬)

@akx akx deleted the ruffify branch December 21, 2023 08:24
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.

2 participants