graphene-django
graphene-django copied to clipboard
👷 Add pre-commit and lint with black and pyupgrade
This adds some basic pre-commit rules and enforcement on CI.
Black gets a version bump and files are formatted.
Pyupgrade is run to upgrade pre 3.6 syntax.
Instead of sending a big chunk of changes, can you please create smaller PRs with only 1 or 2 of the pre-commit rules? This is pretty hard to review as is.
@ulgens +1, or just remove the commit that applies the changes and a maintainer runs pre-commit locally to ensure nothing else was changed by accident. Adding black or the other hooks won't introduce breaking changes.
@ulgens +1, or just remove the commit that applies the changes and a maintainer runs pre-commit locally to ensure nothing else was changed by accident. Adding black or the other hooks won't introduce breaking changes.
Yup, wasn't sure on how to include the actual formatting changes. Reverted the formatting changes so they can be applied by a maintainer instead
@nikolaik @erikwrede If you guys are okay with waiting for 2-3 days, I'll recommend something else.
We should remove Django 2.2 and Python 3.6 from the test matrix, and with their removal, the code should be updated. I can prioritize that cleaning and then we can apply other styling related pre-commit config.
@nikolaik @erikwrede If you guys are okay with waiting for 2-3 days, I'll recommend something else.
We should remove Django 2.2 and Python 3.6 from the test matrix, and with their removal, the code should be updated. I can prioritize that cleaning and then we can apply other styling related pre-commit config.
Good idea! Something like this https://github.com/graphql-python/graphene-django/pull/1337 ?
Hello @nikolaik ! Would you please resolve conflicts so we can merge this?
Hello @nikolaik ! Would you please resolve conflicts so we can merge this?
Hi @firaskafri fixed the conflicts now. There are test failures (which are also present on main) and I left the actual formatting up to a maintainer as suggested.
Hello @nikolaik ! Would you please resolve conflicts so we can merge this?
Hi @firaskafri fixed the conflicts now. There are test failures (which are also present on main) and I left the actual formatting up to a maintainer as suggested.
Hi @nikolaik, fixed the previous change that caused tests to fail, but now seems to be some errors with this PR, can you please help?
Hello @nikolaik ! Would you please resolve conflicts so we can merge this?
Hi @firaskafri fixed the conflicts now. There are test failures (which are also present on main) and I left the actual formatting up to a maintainer as suggested.
Hi @nikolaik, fixed the previous change that caused tests to fail, but now seems to be some errors with this PR, can you please help?
@firaskafri merged with main to fix issues with tests, should be ready to go now!
Hello @nikolaik ! Would you please resolve conflicts so we can merge this?
Hi @firaskafri fixed the conflicts now. There are test failures (which are also present on main) and I left the actual formatting up to a maintainer as suggested.
Hi @nikolaik, fixed the previous change that caused tests to fail, but now seems to be some errors with this PR, can you please help?
@firaskafri merged with main to fix issues with tests, should be ready to go now!
Tests seem to still fail :(
Hi @firaskafri fixed the conflicts now. There are test failures (which are also present on main) and I left the actual formatting up to a maintainer as suggested.
Hi @nikolaik, fixed the previous change that caused tests to fail, but now seems to be some errors with this PR, can you please help?
@firaskafri merged with main to fix issues with tests, should be ready to go now!
Tests seem to still fail :(
Ah, you mean the linter checks. Here you go 81787d9