Use f-strings instead of formatting #2301
Conversation
|
Thanks for your sympathy @oprypin 😅 |
|
This looks good. As an aside, normally I am opposed to doing these kinds of changes. I prefer waiting until PR happens to touch something to make the change. However, the work is already done here and it makes the code so much more readable. Apparently, we are waiting until after #2299 is merged before merging this. I've adding this to the 1.2 milestone which will remind me to merge this before release. If for some reason #2299 is not included in 1.2 I will likely move forward on this anyway. |
|
#2299 has been merged, I'll happily merge this after the conflicts are resolved. |
mkdocs/config/base.py
Outdated
| ) | ||
| elif cfg['strict'] and len(warnings) > 0: | ||
| raise exceptions.ConfigurationError( | ||
| f"Aborted with {len(warnings)} Configuration Warnings in 'strict' mode!" | ||
| raise exceptions.Abort( |
There was a problem hiding this comment.
Note: these look like I'm changing from an f-string to .format, but actually I just decided to drop this change compared to master.
|
Who said about code linting? This is not it. |
Indeed, poor choice of words. That being said, my question still stands here, what if |
|
@ultrabug we are already using flake8. Here's the report from the last run on this issue. And it is one of the tox envs in our tox config. I suspect that is good enough. Currently we only use flake8's defaults (except for line length). While I don't see the need to make any changes, if you see something that could help, then that should be a separate PR from this. Now, to focus on this issue specifically. I'm not sure requiring fstrings is desirable. Yes, in most cases, that is preferred. However, there may be a few occasions where it is not practical. For example, we could have a situation where a template string is generated and later the variables are generated and passed to the template. In that case, a call to format would be required. I'm sure other exceptions could arise. Therefore, I don't see the point in enforcing such a rule with a linter. Good review if PRs should catch these sorts of things. |
I saw in #2283 that you support f-strings.
Well, rather than changing them little by little (whenever a PR happens to touch something) why not change all at once.
Most of these were generated by automation (
pyupgrade+flynt) but I excluded a few and added a few.This is split into commits, ranging basically from ones requiring least caution (especially as the first commit is fully generated) to requiring most caution. So it's recommended to view this as commits, not as a whole.
And I can sympathize with merge conflicts with #2299, so you can leave my PR until that one is merged.