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

replace flake8 sorter with isort #242

Merged
merged 7 commits into from
Sep 4, 2019

Conversation

cs01
Copy link
Contributor

@cs01 cs01 commented Aug 27, 2019

There is no easy way to automatically satisfy flake8's linting requirements for sort order. Using isort there is.

I ran

isort --recursive nox

to automatically sort imports. Then I updated noxfile to test with isort rather than flake8.

@cs01 cs01 mentioned this pull request Aug 27, 2019
Copy link
Collaborator

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

I'm +1 on isort

noxfile.py Outdated Show resolved Hide resolved
@cs01
Copy link
Contributor Author

cs01 commented Aug 28, 2019

Unfortunately isort’s output is not deterministic. It depends on what packages you have installed, your working directory and probably other factors. The build is now failing even though I ran the blacken session locally. I will have to follow up here. One solution may just be to remove the isort check from nox’s lint session. Not sure if it should be left in blacken since it may be different every time someone else runs it.

@theacodes
Copy link
Collaborator

theacodes commented Aug 28, 2019 via email

@cs01
Copy link
Contributor Author

cs01 commented Aug 28, 2019

Removed isort check in lint session.

Copy link
Collaborator

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

👍

noxfile.py Outdated
@@ -59,16 +58,19 @@ def cover(session):
@nox.session(python="3.7")
def blacken(session):
"""Run black code formater."""
session.install("black")
session.run("black", "nox", "tests", "noxfile.py", "setup.py")
session.install("black", "isort==4.3.21")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to only pin isort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pinned it because I thought it would fix the discrepancy between me running it locally and the CI check, but it didn't. Still, it might help keep the sort order more consistent between runs. I don't really feel strongly either way.

@theacodes
Copy link
Collaborator

theacodes commented Aug 28, 2019 via email

@theacodes theacodes merged commit 360c0e8 into wntrblm:master Sep 4, 2019
@cs01 cs01 deleted the cs01/import-sort-order branch September 4, 2019 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants