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

Sort imports based on flake8-import-order #572

Closed
wants to merge 2 commits into from

Conversation

bhrutledge
Copy link
Contributor

As an alternative to #570, and partially out of my own curiosity. I think I prefer isort, but I don't feel strongly.

Changes:

As in the other PR, this allows multiple imports per line, which minimizes the changes, esp. with the typing module.


import keyring

from . import utils
from . import exceptions
Copy link
Member

Choose a reason for hiding this comment

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

We seem to have a mixture of absolute imports from twine import ... and absolute-relative imports from . import ... we should probably standardize on those

Copy link
Contributor Author

@bhrutledge bhrutledge Feb 11, 2020

Choose a reason for hiding this comment

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

Happy to address this after we settle on one of these PRs.

@bhrutledge bhrutledge requested a review from di February 11, 2020 18:11
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

I'd prefer to use isort.

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

I refrain from having an opinion on style until it impedes our contributors' ability to contribute meaningful changes. In the meantime, let it ride.

@bhrutledge
Copy link
Contributor Author

Superseded by #570

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.

4 participants