search

tjcsl / ion

TJ Intranet 3
https://ion.tjhsst.edu
GNU General Public License v2.0
100 stars 89 forks source link

Agree on an ordering for imports #739

Closed theo-o closed 5 years ago

theo-o commented 5 years ago

As discussed in https://github.com/tjcsl/ion/pull/738#discussion_r310305166, we have a disagreement over how to properly order imports.

This issue is meant to collect opinions on the different sides. What we can agree on is that we should split up our imports into related blocks and order the imports alphabetically within in the block. The main question at issue is how the third-party and Django imports should be ordered in general.

P.S.: We should probably enforce whatever we agree on in CI.

/cc @anonymoose2

theo-o commented 5 years ago

@anonymoose2: From my perspective, I think stdlib -> third-party -> Django -> local imports makes the most sense because I see Django as its own distinct category of third-party libraries. I personally think that Django is closer to Ion because it itself uses third-party libraries and standard libraries, although I do see where your line of thinking comes from.

I am also somewhat inspired by Django's style guide (which I guess is sort of in a position of authority):

Put imports in these groups: future, standard library, third-party libraries, other Django components, local Django component, try/excepts.

Unrelated thought on CI

I also think that this is a good CI step:

isort -c $(find intranet -name \*.py ! -path \*migrations\*)
ghost commented 5 years ago

Okay, you have me with the Django style guide. That's authoritative enough for me.

With regards to CI, that sounds good. I'm working on a cleanup/fixes PR, and I can reorder the imports and add the check as part of that. I'll close the issue when it gets merged.