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

chore: introduce black formatting #354

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Conversation

tombh
Copy link
Collaborator

@tombh tombh commented Jul 29, 2023

Black is an opinionated code formatter https://black.readthedocs.io/en/stable/

This introduces a lot of formatting changes.

This has the downside of now assigning all these changes to me, the committer, which means we lose some nuance as to the history and context of the original changes. But maybe it is better to "rip the bandage" to begin enjoying consistent formatting? This isn't actually such a problem because of .git-blame-ignore-revs

TODO:

  • Even though Black is designed to be minimally configurable, there are some options, such as setting the max line length (defaut is 88): https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html
  • scripts/generate_client.py currently outputs non Black-compatible code. We could either make it output compatible code, or just add a compulsory Black formatting step. I think we just add the extra Black format step.

Code review checklist (for code reviewer to complete)

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Commit messages are meaningful (see this for details)
  • Tests have been included and/or updated, as appropriate
  • Docstrings have been included and/or updated, as appropriate
  • Standalone docs have been updated accordingly

@tombh tombh force-pushed the tombh/introduce-black-formatting branch from 6e932f0 to 5a9bee9 Compare July 29, 2023 21:57
@alcarney
Copy link
Collaborator

This has the downside of now assigning all these changes to me, the committer, which means we lose some nuance as to the history and context of the original changes.

I stumbled across something that might help with this a little while ago - .git-blame-ignore-revs it looks like you can configure git to ignore some commits when doing a git blame

Looks like GitHub supports it too

@tombh
Copy link
Collaborator Author

tombh commented Jul 29, 2023

Nice! Added it 😀

@tombh tombh force-pushed the tombh/introduce-black-formatting branch from 3791943 to 8fb554a Compare July 29, 2023 22:25
@dimbleby
Copy link
Contributor

scripts/generate_client.py currently outputs non Black-compatible code

another way would be just not to format the generated code

[tool.black]
exclude = "pygls/lsp/whatever"

@tombh tombh force-pushed the tombh/introduce-black-formatting branch from 8fb554a to e2ea955 Compare July 30, 2023 23:24
@tombh tombh force-pushed the tombh/introduce-black-formatting branch from e2ea955 to 16601a2 Compare July 30, 2023 23:34
@tombh
Copy link
Collaborator Author

tombh commented Jul 30, 2023

Nice idea! Updated and all green CI now, thanks.

@tombh tombh requested a review from alcarney July 30, 2023 23:43
@tombh tombh force-pushed the tombh/introduce-black-formatting branch from 16601a2 to b6c6d74 Compare July 30, 2023 23:44
@tombh tombh merged commit fd1311b into main Jul 31, 2023
@tombh tombh deleted the tombh/introduce-black-formatting branch July 31, 2023 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants