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

More small fixes from downstream projects. #2292

Merged
merged 1 commit into from May 3, 2016

Conversation

Projects
None yet
4 participants
@jmchilton
Copy link
Member

commented May 3, 2016

  • Fixes a bug in lib/galaxy/tools/linters/general.py found through galaxy-lib. (I don't know what id the global does - but I wish flake8 knew I never want to reference it).
  • Brings in docstring and import order linting fixes from the new gxformat2 project (https://github.com/jmchilton/gxformat2).
More fixes from downstream projects.
 - Fixes a bug in lib/galaxy/tools/linters/general.py found through galaxy-lib.
 - Brings in docstring and import order linting fixes from the gxformat2 project (https://github.com/jmchilton/gxformat2).

import yaml
from collections import OrderedDict

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo May 3, 2016

Member

I usually put from foo import bar imports after import foo imports (in each of the 3 recommended subsections), but I can't find an official source for this practice. Anyone has opinions on this?

This comment has been minimized.

Copy link
@martenson

martenson May 3, 2016

Member

I follow this too, but I might heave learned it from you 🐦 👀

This comment has been minimized.

Copy link
@dannon

dannon May 3, 2016

Member

pep8 recommends the ordering stdlib, third party libraries, then local application, but it doesn't explicitly say 'from' comes after direct import that I can find, though I also do follow that usually.

edit: misread the diff slightly, disregard removed point 1 ;)

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo May 3, 2016

Member

It seems this is also the default of https://github.com/public/flake8-import-order (cryptography style).

This comment has been minimized.

Copy link
@jmchilton

jmchilton May 3, 2016

Author Member

I swapped the flake8-import-order from cryptography to google - I prefer cryptography myself but I thought people would like to see the word google. Can we agree as a project to cryptography?

This comment has been minimized.

Copy link
@dannon

dannon May 3, 2016

Member

Now that I look at the 3 side by side, I think I like smarkets best. Second choice would be cryptography.

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo May 3, 2016

Member

+1 to cryptography style (easier to search for me).

This comment has been minimized.

Copy link
@martenson

martenson May 3, 2016

Member

cryptography makes the most sense to me, but I will adapt to whatever you guys decide to use

This comment has been minimized.

Copy link
@jmchilton

jmchilton May 3, 2016

Author Member

Can we move this discussion to #2300 - I'm happy to discuss this but since Galaxy doesn't currently prescribe an import order or offer any linting to do this it would seem inappropriate to delay this PR on import order discussions or concerns.

This comment has been minimized.

Copy link
@dannon

dannon May 3, 2016

Member

@jmchilton Totally agreed.

@martenson martenson merged commit 5063384 into galaxyproject:dev May 3, 2016

4 checks passed

api test Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished.
Details
toolshed test Build finished.
Details

@jmchilton jmchilton referenced this pull request May 3, 2016

Closed

Lint Python Import Order #2300

@jmchilton jmchilton deleted the jmchilton:more_downstream_fixes branch May 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.