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

Does it have to be this specific black version? #24

Closed
MaGering opened this issue Jul 6, 2021 · 3 comments · Fixed by #27
Closed

Does it have to be this specific black version? #24

MaGering opened this issue Jul 6, 2021 · 3 comments · Fixed by #27
Assignees
Labels
question Further information is requested

Comments

@MaGering
Copy link
Collaborator

MaGering commented Jul 6, 2021

Setting up github actions in PR #23, I've specified the version of black to black==20.8b1 orientating on https://github.com/rl-institut/oemof-B3/pull/25/files.

This has led to the following question:


Does it have to be this specific black version?

Originally posted by @jnnr in #23 (comment)


@MaGering MaGering added the question Further information is requested label Jul 6, 2021
@SabineHaas
Copy link
Collaborator

No, the only important thing is that we all have to use the same version of black.

Also interesting, how to set up a hook so that formatting is automatic at push event:

I added a file named pre-commit (no extension) to my local git repo's .git/hooks. The file contains:

#!/bin/sh
flake8 .
black . --exclude /docs

Maybe the order matters?

Then, I made the file executable by chmod +x

Now, each time when I commit, the linters run first and stop the commit if there is mistake.

Originally posted by @jnnr in rl-institut/oemof-B3#25 (comment)

Nice thank you! I guess you could even push that to the repo, so that we can use it, as well.

Now you can use black ., no need for --exclude, as I've added a config file for black, too.

Originally posted by @SabineHaas in rl-institut/oemof-B3#25 (comment)

@jnnr
Copy link
Collaborator

jnnr commented Jul 7, 2021

I had problems with this pre-commit hook: black would reformat, but this would not be added and commited with the same commit. I changed the hook such that it only checks (--check gives an error, other than --diff). Now it looks like this:

#!/bin/sh

flake8 . && black . --check

@jnnr
Copy link
Collaborator

jnnr commented Jul 12, 2021

Can you add an inline comment to the requirement that explains why we fix the black version, @MaGering? After that, I would consider this issue as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants