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

DM-43199: Move to black and isort #91

Merged
merged 5 commits into from Mar 6, 2024
Merged

DM-43199: Move to black and isort #91

merged 5 commits into from Mar 6, 2024

Conversation

mfisherlevine
Copy link
Contributor

No description provided.

setup.cfg Outdated
# N816: mixed case variable in class scope
# W503: line break after binary operator, flake8 disagrees with PEP-8
# W504: Line break occurred after a binary operator
ignore = E133, E203, E226, E228, N802,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think E133 and E226 are impossible once you are enforcing black.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

setup.cfg Outdated
# W504: Line break occurred after a binary operator
ignore = E133, E203, E226, E228, N802,
N803, N806, N812, N813, N815,
N816, W503, W504
Copy link
Contributor

Choose a reason for hiding this comment

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

I have W503 set in middleware packages and that's the one listed in the developer guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless there's a good reason to remove one of these, I'd probably lean towards keeping it, because it can be very annoying battling these sometimes. Is there a good reason to remove W504, do you think? Or is it more of a case of needing one, but not both?

Copy link
Contributor

@timj timj Mar 6, 2024

Choose a reason for hiding this comment

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

You can't have W503 and W504, you have to pick one for flake8 (they mean opposite things). We pick W503 because that matches black.

Copy link
Contributor

Choose a reason for hiding this comment

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

and then I messed up my comment and said W504 by mistake (now fixed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, they're rules to ignore, so I'd have thought you can pick all of them... but sure, will remove W504.

@mfisherlevine mfisherlevine merged commit 9c53239 into main Mar 6, 2024
4 checks passed
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

3 participants