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

Add Black to pre-commit #361

Merged
merged 4 commits into from
Sep 12, 2022

Conversation

erikwrede
Copy link
Member

@erikwrede erikwrede commented Sep 9, 2022

Adds black to pre-commit to ensure a consistent code style over all future contributions

Signed-off-by: Erik Wrede <erikwrede2@gmail.com>
@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #361 (4627a0a) into master (43df4eb) will not change coverage.
The diff coverage is 96.92%.

@@           Coverage Diff           @@
##           master     #361   +/-   ##
=======================================
  Coverage   96.50%   96.50%           
=======================================
  Files           9        9           
  Lines         801      801           
=======================================
  Hits          773      773           
  Misses         28       28           
Impacted Files Coverage Δ
graphene_sqlalchemy/enums.py 97.80% <75.00%> (ø)
graphene_sqlalchemy/types.py 94.35% <92.85%> (ø)
graphene_sqlalchemy/batching.py 95.45% <100.00%> (ø)
graphene_sqlalchemy/converter.py 96.20% <100.00%> (ø)
graphene_sqlalchemy/fields.py 99.15% <100.00%> (ø)
graphene_sqlalchemy/registry.py 100.00% <100.00%> (ø)
graphene_sqlalchemy/resolvers.py 100.00% <100.00%> (ø)
graphene_sqlalchemy/utils.py 93.33% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator

@sabard sabard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible to change:

default_language_version:
  python: python3.10

to

default_language_version:
  python: python3

in .pre-commit-config.yaml? I'm using Python3.8 now and I can't lint without that. I'd make the change, but don't have access to your repo.

The other comments here are just my preference; let me know what you think. Otherwise looks great.

.flake8 Outdated
@@ -1,4 +1,4 @@
[flake8]
ignore = E203,W503
exclude = .git,.mypy_cache,.pytest_cache,.tox,.venv,__pycache__,build,dist,docs
exclude = .git,.mypy_cache,.pytest_cache,.tox,.venv,__pycache__,build,dist,docs,setup.py,docs/*,examples/*,tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason that flake8 should not be run for docs, tests, and examples? When I ran it myself on these it caught a couple import things that should be fixed.

.flake8 Outdated
@@ -1,4 +1,4 @@
[flake8]
ignore = E203,W503
exclude = .git,.mypy_cache,.pytest_cache,.tox,.venv,__pycache__,build,dist,docs
exclude = .git,.mypy_cache,.pytest_cache,.tox,.venv,__pycache__,build,dist,docs,setup.py,docs/*,examples/*,tests
max-line-length = 120
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: If we're changing things here, can we set max-line-length to 79 as per PEP-8? Or maybe there's a good reason for 120?

@@ -1,11 +1,8 @@
[aliases]
test=pytest

[flake8]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we put all flake8 config here instead of having a separate file?

@erikwrede erikwrede merged commit b3657b0 into graphql-python:master Sep 12, 2022
@erikwrede erikwrede deleted the add_black_to_pre_commit branch September 12, 2022 19:37
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 this pull request may close these issues.

None yet

2 participants