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

Extend Python3 flake8 linting to whole codebase #4811

Merged
merged 3 commits into from Oct 16, 2017

Conversation

Projects
None yet
2 participants
@nsoranzo
Member

nsoranzo commented Oct 16, 2017

  • Fix Python3 compatibility and import order for 21 files
  • Add doctests for unicodify()

Keep .ci/py3_sources.txt as list of manually ported files.

xref. #1715

@jmchilton

This comment has been minimized.

Member

jmchilton commented Oct 16, 2017

Wonderful 😀!

Keep .ci/py3_sources.txt as list of manually ported files.

Meaning all files lint as Python 3 but these are the ones you (or someone) have inspected and reviewed as Python 3 compatible (i.e. you've gone through and handled issues that linting doesn't catch)?

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Oct 16, 2017

@jmchilton Yes, flake8 doesn't catch many Python3-compatibility problems, like moved standard library modules, iteritems...
Almost all files in .ci/py3_sources.txt have been ported over the years with the technique explained in the description of #1715.

@jmchilton jmchilton merged commit 24e0c3d into galaxyproject:dev Oct 16, 2017

7 checks passed

api test Build finished. 304 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
selenium test Build finished. 98 tests run, 1 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment