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

Run black on the whole repository #42

Closed
b1rger opened this issue Feb 16, 2023 · 10 comments · Fixed by #43
Closed

Run black on the whole repository #42

b1rger opened this issue Feb 16, 2023 · 10 comments · Fixed by #43

Comments

@b1rger
Copy link
Contributor

b1rger commented Feb 16, 2023

The black documentation writes (emphasis by me):

when migrating your project’s code style to Black, reformat everything and commit the changes (preferably in one massive commit)

and:

Black is strictly about formatting, nothing else. Black strives to ensure that after formatting the AST is checked with limited special cases where the code is allowed to differ. If issues are found, an error is raised and the file is left untouched.

Therefore I propose to just run black on the whole codebase instead of cluttering the git history with small formatting commits.

@SteffRhes
Copy link
Member

Sounds good to me.

b1rger added a commit that referenced this issue Feb 20, 2023
@koeaw
Copy link
Contributor

koeaw commented Feb 22, 2023

I'm very much in favour of this but suggest we don't go with the most recent stable release of Black because it removes all blank lines between comments – which decreases readability in general, and might really mess with our files since there is still lots of (temporarily) commented code in there.

@b1rger
Copy link
Contributor Author

b1rger commented Feb 22, 2023

I'm very much in favour of this but suggest we don't go with the most recent stable release of Black because it removes all blank lines between comments – which decreases readability in general, and might really mess with our files since there is still lots of (temporarily) commented code in there.

Just to clarify: you're referring to version 23.01 as the most recent stable release, but using 22.12 would be ok?

@koeaw
Copy link
Contributor

koeaw commented Feb 22, 2023

Yup, sorry, was going to link issues/comments related to this, but it took me a bit longer than expected to find them again: psf/black#3555 and searxng/searxng#2159 (comment)

@SteffRhes
Copy link
Member

I merged the fat commit yesterday, so feel free and black to whole repo please.

@SteffRhes
Copy link
Member

What's the character line width setting?

@b1rger
Copy link
Contributor Author

b1rger commented Mar 10, 2023

Black defaults to 88 chars and we don't have an override: https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

@SteffRhes
Copy link
Member

It can be overriden: https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

But I don't advocate for it anyway, eventhough I personally prefer 100 as I tend to be verbose with naming.

Since the apis project is moving into a much more collaborative dev setting by now, it's reasonable to stick with defaults.

@return42
Copy link

return42 commented Mar 8, 2024

Yup, sorry, was going to link issues/comments related to this, but it took me a bit longer than expected to find them again: psf/black#3555 and searxng/searxng#2159 (comment)

Just FYI: the issue has been solved since psf/black#4060 has been merged .. was a long discussion .. but finally it becomes good :-)

@b1rger
Copy link
Contributor Author

b1rger commented Mar 8, 2024

Yup, sorry, was going to link issues/comments related to this, but it took me a bit longer than expected to find them again: psf/black#3555 and searxng/searxng#2159 (comment)

Just FYI: the issue has been solved since psf/black#4060 has been merged .. was a long discussion .. but finally it becomes good :-)

Thanks a lot for the notification! Much appreciated!

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 a pull request may close this issue.

4 participants