Skip to content
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

Format the whole repo with Black #419

Merged
merged 12 commits into from
Aug 16, 2023

Conversation

tkrabel-db
Copy link
Contributor

@tkrabel-db tkrabel-db commented Aug 15, 2023

What is changed in that PR?

Fixes #392, i.e. this PR auto formats the whole repo using black to see the diff.

black is arguably the most popular formatter out there.

What needs to be changed further?

Some black re-formattings violate pylint and pycodestyle, e.g.

pycodestyle

# All errors are of this form
test/test_text_edit.py:27:9: W503 line break before binary operator
...

pylint pylsp

************* Module test.test_workspace
test/test_workspace.py:240:0: W1404: Implicit string concatenation found in call (implicit-str-concat)
test/test_workspace.py:259:0: W1404: Implicit string concatenation found in call (implicit-str-concat)
************* Module pylsp.plugins.pylint_lint
pylsp/plugins/pylint_lint.py:292:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
pylsp/plugins/pylint_lint.py:294:0: I0021: Useless suppression of 'consider-using-with' (useless-suppression)
************* Module pylsp.plugins.rope_autoimport
pylsp/plugins/rope_autoimport.py:33:0: R0911: Too many return statements (7/6) (too-many-return-statements)
pylsp/plugins/rope_autoimport.py:35:0: I0021: Useless suppression of 'too-many-return-statements' (useless-suppression)

My suggestion:

  • blacktakes care of code styling, so we can remove pycodestyle altogether.
  • pylint goes deeper, as it also e.g. cares about import order, so I suggest the user needs to fix these manually

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tkrabel-db for your help with this!

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/static.yml Outdated Show resolved Hide resolved
.github/workflows/static.yml Outdated Show resolved Hide resolved
.github/workflows/static.yml Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 added this to the v1.8.0 milestone Aug 15, 2023
@ccordoba12 ccordoba12 changed the title black format the whole Repo Format the whole repo with Black Aug 15, 2023
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now, thanks @tkrabel-db!

@ccordoba12 ccordoba12 merged commit d47dc3c into python-lsp:develop Aug 16, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using black as an autoformatter
2 participants