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

widen black formatter (88 -> 100 chars) #97

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

claforte
Copy link
Collaborator

@claforte claforte commented Jun 3, 2023

@bennyguo this is a very simple way to enforce a non-default max width. I ran some quick tests with a relatively large font in vscode, 2 files side-by-side plus the bar on the left, and I recommend a value between 100 and 110.

The moment we merge this however, we might want to do a project-wide reformatting of the python files. If we don't, when someone pushes a commit, their actual change will be burried in tons of unrelated formatting changes. sigh. I volunteer to figure this out and do this right after the PR is merged, if you agree with this plan.

@thuliu-yt16 @voletiv FYI. If you have large outstanding changes, I recommend you merge them before we do this.

@claforte claforte requested a review from bennyguo June 3, 2023 15:50
@bennyguo
Copy link
Collaborator

bennyguo commented Jun 3, 2023

Tested. It works for me. BTW I modify the pre-commit config so that pre-commit could also adapt to this new config.

@bennyguo
Copy link
Collaborator

bennyguo commented Jun 3, 2023

I think we have a perp-neg branch which seems to have major changes. Let's adapt to the new line length after merging it. Doing a project-wide reformatting is easy: simply run pre-commit run --all-files multiple times until there're no errors.

@bennyguo bennyguo marked this pull request as draft June 5, 2023 12:18
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.

2 participants