-
-
Notifications
You must be signed in to change notification settings - Fork 284
Add pre-commit hooks enforcing a standard style #28
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
Conversation
rev: v5.8.0 | ||
hooks: | ||
- id: isort | ||
args: ['--multi-line=3', '--trailing-comma', '--force-grid-wrap=0', '--use-parentheses', '--line-width=88'] |
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.
Calling this out because the settings look strange at first.
These are the recommended settings for interop with black (which also may adjust imports)
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.
Makes sense. Thanks for pointing this out! Just going to plop the reference doc here for my own reference.
The black hook is going to cause conflicts any time other stuff merges into develop before this PR. I can strip out the commit that applies the changes and limit this PR to just the pre-commit config, CI, and README files if you prefer. The CI hooks would fail after merge but you'd be able to apply the hooks locally when you're ready. Would that be easier to deal with? |
Hey @olirice, Thanks for considering the downstream consequences of this PR, it is really thoughtful of you. No worries, I'll look into this Pull Request in a few hours so that we can get this merged before anything else gets merged. Joel |
LGTM. Thanks much! |
What kind of change does this PR introduce?
Enforces code consistency via pre-commit hooks and checks pushed code on all branches in CI
I left off the mypy check for now because there were some tricky type errors in the query builder
Resolves #26