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

Should isort be included as default lint target? #29

Open
xiang-zhu opened this issue Apr 20, 2020 · 2 comments
Open

Should isort be included as default lint target? #29

xiang-zhu opened this issue Apr 20, 2020 · 2 comments

Comments

@xiang-zhu
Copy link

isort seems having compatibility issues with black psf/black#333 PyCQA/isort#694

There's a certain config to work around the issue, and implemented here through #27

I want to start this as dicussion of is isort provide enough value that we want to take on the maintenance responsibility for the incapability and potential breaking change in the future?

@charlieparkes
Copy link

The whole point of build harness is that we control for potential breaking changes by centralizing all the commands we run in one place. If isort+black end up with a serious issue again in the future (issue #27 was not serious), we can remove isort or adjust the commands as necessary.

That said, I think there's potential to drop isort from the lint target (leave it in the fmt target) with the addition of flake8. I'll investigate #28 before returning to this issue.

@charlieparkes
Copy link

charlieparkes commented Apr 20, 2020

Ok, so #31 set the following order-of-operations.

Lint

  1. autoflake - delete unused imports and variables, if possible
  2. isort - clean up imports (ordering, formatting)
  3. black - "no opinions allowed, this is the way"
  4. flake8 - list style and quality errors that impact readability and/or are the source of bugs

All of these things blend together well and do something the others don't. I limited each to do the bare minimum, where possible, so there's very little duplicate work. (isort and black may try to duplicate each-other, but it shouldn't cause any conflicts in the current configuration)

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

No branches or pull requests

2 participants