-
Notifications
You must be signed in to change notification settings - Fork 21
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
Apply black
formatting
#270
Conversation
len(title) | ||
+ len(rest_of_filename) | ||
- MAXIMUM_FILENAME_LENGTH | ||
len(title) + len(rest_of_filename) - MAXIMUM_FILENAME_LENGTH |
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.
🤔 . I think I would keep the way it was. I understand it fits on one line with the parenthesis. But it matched our wish to have operators on the next line.
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.
We'll have to keep fighting with black
if we do that 😢
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.
black
is not the almighty ;-)
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.
The issue is that we cannot then run black
across the codebase or as a pre-commit hook if we fight things like this. It would be better not to use black
at all then which is still a valid option
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.
The issue is that we cannot then run black across the codebase or as a pre-commit hook
That's exactly my point. The decision should be ours to take. I have to admit that I'm not a big fan of using black
a pre-commit hook.
104fb1d
to
6183f2b
Compare
@joshuaberetta, please remove the black dependency and open an issue to add it when we have a consensus about it. |
Related issues
closes #268