Skip to content

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.

akx added 13 commits August 15, 2023 13:36
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
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
@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