-
Notifications
You must be signed in to change notification settings - Fork 27
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
Improve formatting #236
Improve formatting #236
Conversation
Oh wow, thanks so much for doing this. Yeah, there were an absurd amount of things that needed to be linted here. |
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.
@sommersoft I can't thank you enough for doing this. Last time I checked Pylint had about 700 things that it wanted changed, I can't imagine how long it took.
Testing the scripts:
|
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.
This is brilliant, thank you so much for doing it! @dherrada is testing it locally to ensure everything is still copacetic.
@sommersoft I had one question below. I noticed you pinned Black, and I wasn't sure if we should be pinning Pylint as well.
@@ -1,3 +1,4 @@ | |||
black==21.6b0 | |||
packaging==20.3 | |||
pylint |
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.
Should this be pinned as well?
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.
I noticed you pinned Black, and I wasn't sure if we should be pinning Pylint as well.
I didn't pin it since the locally installed module is used to verify that a library is passing with the latest pylint, in lib/circuitpython_library_validators.py::validate_passing_linting
.
However, in looking back at changes to libs/cookiecutter, it seems they're all now pinned to 2.7.1 via pre-commit-config. Which brings the question: is validate_passing_linting
overachieving by linting with the latest version?
Ok. Just finished running all 4 of the scripts that we can run locally and they all worked as expected. |
I'm ok with merging this and if we were supposed to pin Pylint, we can handle it in another PR. Thank you for testing, Dylan! Thanks so much for this whole thing, sommersoft! |
Fixes #225.
This is a large one, so apologies for the heavy review load.
Changes include the following:
Ran Black on all Python files
Ran Pylint and addressed most issues. There are some
pylint: disable
markers that can be cleaned up, but would be better done outside this PR.Actions workflow changes:
Beyond automated tests, the following were tested (the rest are currently hard to test):
-o
)--dry-run
&--dry-run --local
)-o
)I've got a small list of new issues to address some of the
pylint: disable
markers, and some other stuff I came across. I'll get those filed soon.