-
Notifications
You must be signed in to change notification settings - Fork 953
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
Reformat code using Black formatter #432
Conversation
aa396e1
to
5de11bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! This looks great. I've added black to the requirements.txt file and added a bit in the documentation. Feel free to make changes to this if you think it can be done better @MartinThoma.
For documentation purposes: this pr has breefly been discussed here
I tried running black on this branch and it formatted the code differently from what is proposed here. I guess running black should result in no changes after this pr. Any idea what could cause this?
And can you add yourself to AUTHORS.rst? Thanks! |
@SijmenHuizenga The difference is the only thing that you can configure in black - the line length 😄 I ran it with "black --line-length 79 ." which is compatible to PEP8. You might have run just "black ." (as in the docs) which by default is a line length of 88 characters. Which line length would you like to have? I could also add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding it to the requirements-dev looks good; the docs might need --line-length
Thanks for the explanation! Since we are changing the formatting anyway we might as well go with the default black settings 😄 The 88 character line width seems more in line with the rest of the world. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
One last thing before we merge, can you please remove the commits that were pulled in from master? Some 'merge' commits are fine but now somehow there are all these commits in here that are unrelated and have you as co-author. If you can't figure out how to fix this git magic, don't worry, just let me know and I will fix it. |
I'm sorry, I don't know that. I typically squash before I merge (or amend + force-push on feature branches). If it's not a big deal for you, please go ahead and do it :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
$ pip install black -U $ black --line-length 79 .
ee45df4
to
51fec42
Compare
No description provided.