-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Update codebase to Python >= 3.6 #2612
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
Conversation
Note that in #2301 I already went over all the occurrences of |
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.
To give a clearer comment...
Now I will point out the particular places where subjectively I think the code "after" is harder to understand than the code "before".
Sorry that I'm not giving the reasoning.
For an easy way forward, maybe you could just undo those parts. The rest is all fine.
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.
Further discussion is also possible.
Certainly there's a lot of merit to setting a clear guideline once and for all.
Some examples of guidelines:
- Always use f-strings, not
.format
(what seems like you're trying). - Use
.format
if one of the items isn't a plain variable or member access
(seems like the current situation, I don't mind it). - If one of the items isn't a plain variable or member access, first assign it to a variable and then always use f-strings anyway
(I could be open to this, but I think it can also be messy).
The last commit - I don't think it's good to clump it together with this pull request. Please drop it. Also not sure about "Rewrite Nav config type validation error message" - ideally would be separate |
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.
Thanks. And separately thanks for your patience :)
Thanks a lot! |
I'm stupid and merged in the wrong order @mondeja , I apologize for the conflict here... |
No problem, it's fixed now 👍🏼 |
.format
calls with f-strings.'r'
mode argument foropen
function as it is uneeded, but maybe maintainers want to keep it explicit?__future__
import andcoding: utf-8
comment from a file.