-
Notifications
You must be signed in to change notification settings - Fork 488
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
Updates KMK's code style in line with Black's double quotes #571
base: master
Are you sure you want to change the base?
Conversation
c381643
to
211b9ef
Compare
211b9ef
to
c218396
Compare
isort config updated with latest libraries documented by Adafruit.
5aa201e
to
44b9b82
Compare
So this PR seems ready to me. All formatting and tests pass on Win11 with Python 3.10 and Fedora-WSL with 3.9. |
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.
Think this covers my thoughts. Obviously, all PR's will need to conform to this, which will make any of them fail, ect, but other than notes, if pushing through pure black formatting is considered a better approach, then those comments are all that I have before I won't block a merge.
@@ -1,58 +0,0 @@ | |||
[flake8] |
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.
Just making sure it is intentional to delete this file. I presume that just "taking black defaults" is what you expect to do, including ignoring all of the convenient exceptions? Not refuting it, just making sure that we all know what the intent here is.
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.
Intentional.
Only Flake8 remains that doesn't use pyproject.toml. The way I was reading that is they were waiting for a parser to make the standard library (3.11) or for a clear standard to emerge.
Putting flake8 config in it's own file makes it clear (at least to me) that no new configs should be declared in the legacy setup.cfg, preferring pyproject.toml where possible.
Something for the contributing.md, will add if you agree.
Other thing that totally got left out. Docs. They are all still single quote, and need to be updated to reflect the new style, or we cause confusion for users. |
Changes requested in PR571 Doc changes still to come.
Once docs are resolved, I won't block merger, and allow any other maintainer to sign off and merge. |
To Do:
|
I've not forgotten this PR, I just haven't been able to give it my attention. Next week I'll be between $Jobs so I'll be able to get it across the line. |
When you get back (no rush!) let's also update the README to match, since the mainline readme still talks about this vestigial choice |
In #561 I raised the possibility of fully complying with black. The maintainers tended to agree that double-quotes are fine, as long as it's consistent everywhere.
To that end, I've run black and isort across the entire project. Then got flake8 and unittests to all pass.
IMO contributors should use block exclusions for keymaps, et al rather than entire files. This should help newcomers see a consistent style for the parts of KMK they look at most - namely docs/, keymaps/ and boards/**/main.py
I also took the opportunity to move the remaining isort configs into pyproject.toml. The remaining flake8 configs into a named file. This will probably not move into pyproject.toml until Python 3.11 which adds a toml parser to the standard library.
I've also added a git_ignore_revs file so
git blame
won't blame me for style changes ;) Seems to work in the Github diff UI natively.