import sorting in python code

[John Sirois]

Our import sorting is now automated via isort [1] and checked in ci.

You may see an error like so:

Given:

$ git diff

diff --git a/src/python/pants/binary_util.py b/src/python/pants/binary_util.py

index ab5df55..590c1db 100644

--- a/src/python/pants/binary_util.py

+++ b/src/python/pants/binary_util.py

@@ -5,10 +5,10 @@

 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,

                         unicode_literals, with_statement)

 

-import os

 import posixpath

 import subprocess

 from contextlib import closing, contextmanager

+import os

 

 from twitter.common import log

 from twitter.common.collections import OrderedSet

A CI run will say:

$ ./build-support/bin/ci.sh

...

ERROR: /home/jsirois/dev/3rdparty/pants/src/python/pants/binary_util.py Imports are incorrectly sorted.

To fix import sort order, run `build-support/bin/isort.sh -f`

And you just follow the instructions and run`build-support/bin/isort.sh -f` to fix imports up.

So you may want to get in the habit or running the isort fix early and often before even posting RBs or pushing pull request CI branches.

[1] https://github.com/timothycrosley/isort

[Benjy Weinberger]

Nice! Can we put some sort of wrapper script around this? I don't want to force everyone to remember that magic incantation (specifically that the thing is called 'isort' and requires the '-f' flag).

[Eric Zundel Ayers]

I hit this and the CI build fails pretty early saying, "run build-support/bin/isort.sh -f" pretty clearly so its easy to find and fix (didn't seem like magic to me)

[John Sirois]

You can do this - sure.  I think the best approach though is to impliement a commit hook and add docs to the dev center on how to install the local commit hook.  This will make all checks available via 1 step no matter what those checks are called going forward.

[Brian Wickman]

For examples, you can see what we do in the Aurora repo (purely opt-in):

https://github.com/apache/incubator-aurora/blob/master/build-support/hooks/pre-commit

[Benjy Weinberger]

Word to the wise: isort will fail the CI not only on incorrectly sorted imports but on files that don't end in a newline.

[John Sirois]

On Mon, Feb 9, 2015 at 6:55 PM, Benjy Weinberger <be...@foursquare.com> wrote:

Word to the wise: isort will fail the CI not only on incorrectly sorted imports but on files that don't end in a newline.

On Mon, Feb 9, 2015 at 12:30 PM, Brian Wickman <wic...@twitter.com> wrote:

For examples, you can see what we do in the Aurora repo (purely opt-in):

https://github.com/apache/incubator-aurora/blob/master/build-support/hooks/pre-commit

On Mon, Feb 9, 2015 at 12:29 PM, John Sirois <john....@gmail.com> wrote:

You can do this - sure.  I think the best approach though is to impliement a commit hook and add docs to the dev center on how to install the local commit hook.  This will make all checks available via 1 step no matter what those checks are called going forward.

On Mon, Feb 9, 2015 at 1:26 PM, Benjy Weinberger <be...@foursquare.com> wrote:

Nice! Can we put some sort of wrapper script around this? I don't want to force everyone to remember that magic incantation (specifically that the thing is called 'isort' and requires the '-f' flag).

Its still a magic incantation, but its documented and consolidated/stable now at least: https://rbcommons.com/s/twitter/r/1883/