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

better isort defaults, rerun, reblacked #1923

Merged
merged 3 commits into from
May 28, 2018
Merged

Conversation

nicpottier
Copy link
Collaborator

@nicpottier nicpottier commented May 28, 2018

Trailing comma in black is the only difference now, could be worth doing this until a new version of black comes out:
psf/black#250

setup.cfg Outdated
force_grid_wrap = 0
line_length = 119
combine_as_imports = True
sections = FUTURE,STDLIB,THIRDPARTY,DJANGO,DJANGOTHIRDPARTY,FIRSTPARTY,LOCALFOLDER
Copy link
Member

Choose a reason for hiding this comment

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

not sure I'd bother splitting third party into THIRDPARTY,DJANGO,DJANGOTHIRDPARTY but no biggie

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure it is actually being triggered anywhere

Copy link
Member

Choose a reason for hiding this comment

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

all the files I'm seeing now have django.* imports in their own section

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh thought you were talking third party django vs django. I for one like having Django being in their own thing.

This is why we shouldn't have options!

@nicpottier
Copy link
Collaborator Author

Found that with trailing_comma option they now agree. I think that ticket only applies for super long top level package names which we don't run into, so I think we can merge this as is.

@nicpottier nicpottier merged commit 259e873 into master May 28, 2018
@nicpottier nicpottier deleted the better-isort-defaults branch May 28, 2018 17:18
@nicpottier nicpottier removed the review label May 28, 2018
combine_as_imports = True
sections = FUTURE,STDLIB,THIRDPARTY,DJANGO,DJANGOTHIRDPARTY,FIRSTPARTY,LOCALFOLDER
known_django = django
known_djangothirdparty = celery, leaflet

Choose a reason for hiding this comment

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

I think smartmin, DRF, django-countries should also belong in the django third party

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.

3 participants