search

Skyscanner / skyscanner-python-sdk

Skyscanner Python SDK
Other
122 stars 33 forks source link

add flake8 in build check, reorder imports #17

Closed kelvintaywl closed 8 years ago

kelvintaywl commented 8 years ago

This PR updates the SDK such that:

  1. introduce flake8 as part of Travis CI build to ensure that we follow PEP8 guidelines as close as possible.
  2. reordering of imports to follow import guidelines from PEP8 (ref: https://www.python.org/dev/peps/pep-0008/#imports).

I have explicitly set flake8 config to ignore max line length (defaults at 79 chars) though. perhaps we want to enforce a specific max line length?

ardydedase commented 8 years ago

Hi @kelvintaywl,

Let's stick to the standard 79 characters. Also, the build has failed. Could it be because it failed the flake8 check?

Thanks for this!

Regards, Ardy

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.9%) to 92.308% when pulling b446deea40eb440949c47eb6ee90f15a0013a612 on kelvintaywl:add-flake8-reorder-imports into 0dd7eec6410c9dc7ecfa3509de6db2620844d622 on Skyscanner:master.

kelvintaywl commented 8 years ago

Hi @ardydedase

Thanks for the response! I have set it to follow the recommended 79 char limitation then. As a result, I have also formatted the codes to fit the 79-char limitation.

A small note, however, as I have elected to ignore the E402 rule from PEP8 as there seem to be a conflict between flake8's handling of checking E402 against dunder names such as __author__, __version__, etc.

build is currently failing as flake8 does not support python 2.6. I'm not sure if there's a strong motivation to support python 2.6 though.

ardydedase commented 8 years ago

Hi @kelvintaywl,

The changes are looking good. Noted the E402 rule as well. I'm now considering ditching Python 2.6. It also causes us syntax problems sometimes. Thanks a lot for your contribution!

Regards, Ardy