Introduce better coding style #171
Conversation
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
There was a problem hiding this comment.
Thanks @Superskyyy
BTW, @Superskyyy can you help to set up the quote style in flake8? I see we use double quotes (") somewhere and single quotes (') elsewhere, I'd like see them consistent. WDYT?
I'm also wondering whether we can have flake8 rules to check there is no more old-style string manipulation other than f-string?
|
|
https://github.com/zheller/flake8-quotes I'm going to try this one. That worked, got a mountain of code to fix.. AFAIK single quotes are generally more preferred. |
|
Let's wait a bit, almost done, got automation. |
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
- intentional lint failure Signed-off-by: Superskyyy <superskyyy@outlook.com>
Signed-off-by: Superskyyy <superskyyy@outlook.com>
|
Please add this to the ASF repo control file. The approval seems not dismissed. https://github.com/apache/skywalking/blob/master/.asf.yaml#L49 |
Signed-off-by: Superskyyy <superskyyy@outlook.com>
Done |
|
@kezhenxu94 It should be good now. Lint enforcement for quotes added, fixed existing. 💀 I noticed the renaming of tests brought ~6k LOC to my credit, sorry about that... |
Superskyyy
changed the title
Signed-off-by: Superskyyy <superskyyy@outlook.com>
|
Ah, I forgot about the f-string check.. adding now. |
Signed-off-by: Superskyyy <superskyyy@outlook.com>
|
More rules coming in next PR, have some more cleanup to do. |
|
It is great to have these cleanup before our important 1.0.0 release! |
| Please do not use other styles - `+`, `%` or `.format` unless f-string is absolutely unfeasible in the context, or | ||
| it is a logger message, which is [optimized](https://docs.python.org/3/howto/logging.html#optimization) for the `%` style | ||
|
|
||
| Run [flynt](https://github.com/ikamensh/flynt) to convert other formats to f-string, pay **extra care** to possible corner |
There was a problem hiding this comment.
What about adding this to a new Makefile target, for example fix-lint?
There was a problem hiding this comment.
What about adding this to a new Makefile target, for example
fix-lint?
NVM, let's do this in following PRs
There was a problem hiding this comment.
Ok, I have another PR coming. Was taking a nap :)
| bar = f"This repo is called 'skywalking-python'" | ||
| ``` | ||
|
|
||
| Run [unify](https://github.com/myint/unify) `unify -r your_target --in-place` to fix your quotes if flake8 complaints about it. |
There was a problem hiding this comment.
What about adding this to a new Makefile target, for example fix-lint?
Never mind, it's OK because GitHub counts renamed files' lines in contribution graph but the renaming doesn't change the git blame, we can track the changes still. |
This PR migrates existing usage of old-style string manipulation (
%,.formatand+) to f-strings for better clarity and some performance boost.Adds stricter coding style.
Optimizes debug messages for performance.
Signed-off-by: Superskyyy superskyyy@outlook.com