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

Add black to ethpm #2345

Merged
merged 1 commit into from
Jun 15, 2022
Merged

Add black to ethpm #2345

merged 1 commit into from
Jun 15, 2022

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Feb 11, 2022

What was wrong?

Black is finally stable, so pulling it in. One module at a time.

Related to Issue #1416

How was it fixed?

added the Black dependency, let it make any fixes.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@wiseaidev
Copy link
Contributor

How about using pre-commit to automate formatting stuff?

@wiseaidev wiseaidev mentioned this pull request Feb 20, 2022
22 tasks
@kclowes
Copy link
Collaborator Author

kclowes commented Jun 9, 2022

How about using pre-commit to automate formatting stuff?

@Harmouch101 That looks like a good option, but I'm going to hold off for now. If we decide running black is too clunky then I may pull it in. Thanks for the suggestion!

@kclowes kclowes changed the title [WIP] Add black to ethpm Add black to ethpm Jun 9, 2022
@kclowes kclowes requested review from fselmo and pacrob June 9, 2022 21:52
@kclowes kclowes mentioned this pull request Jun 9, 2022
1 task
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

@fselmo
Copy link
Collaborator

fselmo commented Jun 10, 2022

I was originally thinking we could set the line-length to 100 since that's been our standard for some time but I don't feel strongly about that. It still produces a lot of diff (even more I think). Maybe we should just let black do its thing?

This change would do that within pyproject.toml:

[tool.black]
line-length = 100

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm! Excited for this addition. I just left one comment to think about before we start changing everything to black across the code base. But maybe we should just leave it all default black settings since that's kind of the point 😅 .

@pacrob
Copy link
Contributor

pacrob commented Jun 10, 2022

... Maybe we should just let black do its thing?

+1 for default line length 88

@kclowes
Copy link
Collaborator Author

kclowes commented Jun 15, 2022

Okay, I'll leave line length at 88. Thanks!

@kclowes kclowes merged commit 350126e into ethereum:master Jun 15, 2022
@kclowes kclowes deleted the add-black branch June 15, 2022 20:28
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.

4 participants