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

Fix flake8 and tox dependencies #198

Merged
merged 9 commits into from
Sep 28, 2022
Merged

Fix flake8 and tox dependencies #198

merged 9 commits into from
Sep 28, 2022

Conversation

brylie
Copy link
Member

@brylie brylie commented Sep 28, 2022

Closes #196
Closes #197

Changes

  • use latest published versions of flake8 and tox

References

@brylie brylie requested a review from jezdez September 28, 2022 08:20
@codecov
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Merging #198 (11015be) into master (31a2832) will decrease coverage by 0.83%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
- Coverage   64.64%   63.81%   -0.84%     
==========================================
  Files          17       17              
  Lines         478      467      -11     
==========================================
- Hits          309      298      -11     
  Misses        169      169              
Impacted Files Coverage Δ
invitations/models.py 75.00% <ø> (ø)
invitations/urls.py 100.00% <ø> (ø)
invitations/adapters.py 32.83% <100.00%> (-0.99%) ⬇️
invitations/admin.py 62.50% <100.00%> (-4.17%) ⬇️
invitations/forms.py 44.23% <100.00%> (-2.07%) ⬇️
invitations/views.py 57.26% <100.00%> (-1.76%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@brylie brylie marked this pull request as draft September 28, 2022 08:48
@brylie
Copy link
Member Author

brylie commented Sep 28, 2022

I am converting this to draft since pre-commit is causing some hassle. I am also disabling auto fix on pre-commit since it is causing issues with local branch sync.

@brylie brylie marked this pull request as ready for review September 28, 2022 09:01
@brylie
Copy link
Member Author

brylie commented Sep 28, 2022

OK, this is ready for review. Hoever, I'm unsure why the test coverage is dropping since most changes are lint.

@brylie brylie requested a review from valberg September 28, 2022 09:03
args: [--max-line-length=88]
- repo: https://github.com/asottile/reorder_python_imports
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this since it is already handled by isort

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not entirely true - isort does not seperate imports into multiple lines: https://github.com/asottile/reorder_python_imports#why-this-style

I vote we replace isort with reorder_pyton_imports.

@@ -52,12 +50,4 @@ repos:
- repo: https://github.com/asottile/yesqa
rev: v1.4.0
hooks:
- id: yesqa
- repo: https://github.com/asottile/add-trailing-comma
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this since it is handled by black.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not entirely. Black does not add trailing commas when the arguments include *, *args or **kwargs.

I vote we keep add-trailing-comma

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, we can add it in a follow-up PR

@brylie
Copy link
Member Author

brylie commented Sep 28, 2022

@jezdez, could you take a look at this PR?

Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

LGTM!

@brylie brylie merged commit 7612ff1 into master Sep 28, 2022
@brylie brylie deleted the fix-dependency-errors branch September 28, 2022 10:58
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.

flake8 version mismatch when running poetry install Pre-commit hooks failing
3 participants