-
Notifications
You must be signed in to change notification settings - Fork 249
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: use fstrings #250
chore: use fstrings #250
Conversation
8e52e87
to
16ae872
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's quite a few places where the f-strings are harder to read than the original syntax. This occurs when the 'structure' of the original message is obscured by the long, inline code.
I commented on the first few, but there's a lot more places here where i think this holds.
In those cases, I would suggest improving clarity by adding intermediate calculations of the message components, and then using the local variables.
the point of this lint is to make things easier to grok, so we need to be careful about cargo-culting in the lint, and missing the point of why we're using it in the first place
09b4d1c
to
5999359
Compare
thanks @danieleades, hopefully this is more readable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after one tiny correctness fix! Awesome work!
5999359
to
d3660d3
Compare
Towards python-poetry/poetry#4776