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

Setup pre-commit black'ing and linting similarly to dandi-cli #35

Closed
yarikoptic opened this issue Jun 16, 2021 · 5 comments · Fixed by #39
Closed

Setup pre-commit black'ing and linting similarly to dandi-cli #35

yarikoptic opened this issue Jun 16, 2021 · 5 comments · Fixed by #39
Assignees

Comments

@yarikoptic
Copy link
Member

No description provided.

@jwodder
Copy link
Member

jwodder commented Jun 16, 2021

@yarikoptic It is set up; the configuration is copied directly from dandi-cli. In what way is it not working appropriately?

@yarikoptic
Copy link
Member Author

@yarikoptic It is set up; the configuration is copied directly from dandi-cli.

d'oh -- my bad, indeed it is there and I see the pre-commit hooks in .git/hooks.

In what way is it not working appropriately?

for #34 I managed to do locally commit and then push to only get linters unhappy with fixes to follow in f129f15 (unused imports, comments format), and a271084 (too long lines).

May be there is a quick way to just run tox -e lint as part of the precommit to at least ensure (but IIRC for dandi-cli I have black splitting long lines etc) that I am not committing with a lint?

@satra
Copy link
Member

satra commented Jun 17, 2021

just an fyi. this is the pre-commit file i'm using in nobrainer and this will take care of a lot of the issues. https://github.com/neuronets/nobrainer/blob/master/.pre-commit-config.yaml
(that repo also has the precommit-ci setup).

@jwodder
Copy link
Member

jwodder commented Jun 17, 2021

@yarikoptic Black just does formatting; it doesn't care about unused imports. It also, for some reason, doesn't edit comments beginning with multiple #'s despite those sometimes being rejected by flake8. As for long lines, if they're long because they contain a long string, black won't break the string up (but I think that might be on black's to-do list).

One wouldn't run tox itself through pre-commit; one would instead directly run the lint commands that tox runs.

@yarikoptic
Copy link
Member Author

ok, could we run the lint command we use for tox?

yarikoptic added a commit that referenced this issue Jun 18, 2021
Add flake8 to .pre-commit-config.taml
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.

3 participants