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

Change pep8 to flake8. #1394

Merged
merged 1 commit into from Jun 23, 2015

Conversation

Projects
None yet
4 participants
@allisonvacanti
Contributor

allisonvacanti commented Jun 16, 2015

Removed the existing pep8 test -- will readd it as a flake8 test in a later PR.

@doutriaux1 Which modules did you want tested again? I think we said 'everything under Packages/, but that includes things like vcs_legacy and a ton of other modules that I'm not sure are under active development, or are even used anymore.

Here's the full list of modules with the number of flake8 issues detected. Let me know which you want flake8 coverage for.

  • AutoAPI: 417 flake8 issues found.
  • browser: 48017 flake8 issues found.
  • cdms2: 18353 flake8 issues found.
  • cdtime: 35 flake8 issues found.
  • cdutil: 6492 flake8 issues found.
  • cmor: 10766 flake8 issues found.
  • dat: 0 flake8 issues found.
  • demo: 3396 flake8 issues found.
  • distarray: 257 flake8 issues found.
  • DV3D: 16964 flake8 issues found.
  • eof_workbench: 87 flake8 issues found.
  • esg: 555 flake8 issues found.
  • genutil: 7897 flake8 issues found.
  • globus: 24 flake8 issues found.
  • gui_support: 122 flake8 issues found.
  • help: 463 flake8 issues found.
  • kinds: 129 flake8 issues found.
  • lats: 885 flake8 issues found.
  • ncml: 243 flake8 issues found.
  • Properties: 194 flake8 issues found.
  • psql: 22 flake8 issues found.
  • pydebug: 2940 flake8 issues found.
  • regrid2: 1802 flake8 issues found.
  • reqm: 78 flake8 issues found.
  • Thermo: 1459 flake8 issues found.
  • unidata: 310 flake8 issues found.
  • vcs: 107263 flake8 issues found.
  • vcsaddons: 1646 flake8 issues found.
  • vcs_legacy: 55394 flake8 issues found.
  • visus: 14189 flake8 issues found.
  • WK: 1333 flake8 issues found.
  • xmgrace: 5509 flake8 issues found.

@doutriaux1 @chaosphere2112 @aashish24 Please review/merge as soon as possible so I can start pushing test PRs.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 16, 2015

@dlonie AutoAPI and vcs_legacy are gone as soon as you approve my PR 😉
Properties, browser,gui_support, kinds lats and ncml should be removed as well.
visus is not built (yet)
reqm and eof_workbench I have no idea what that is....
help and demo should be moved out by @chaosphere2112 and put on tutorials (it is not built now)
globus and esg are used by the esgf team, not us

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 16, 2015

So which should be tested? All of the others?

Change pep8 to flake8.
Removed the existing pep8 test -- will readd it as a flake8 test
in a later PR.
@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 22, 2015

@dlonie I think so.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2015

@dlonie yes all of the others. But again, submitting in multiple PR would be great so we can slowly merge them one at a time

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 22, 2015

@doutriaux1 Cool. That's the plan -- once this PR is in I'll start setting up tests for the following modules:

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2015

yep. I think dat is empty nowadays. No?

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 22, 2015

That's why I'm asking what you want covered -- I don't know what any of these are.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2015

so yes that list but not dat there's no python in there anyway

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

Ok, updated list.

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

Anything holding up this PR?

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

So I guess in this branch we are not using flake8 for testing. Is that correct?

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

Correct -- this just sets up the superbuild, and then I'll add tests. I can't add them yet because then tests would start failing when this gets merged, and this needs to be merged so I can base the other PRs off of it.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

there seems to be some weird issue on test-laptop (I will check) but other than that it looks good to me.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

LGTM

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

The failure looks related to fetching the flake8 sources. Maybe the host was down?

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

It is possible. I guess in the future, LLNL can host these sources as well.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

I think if that's the case, then we should merge this.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 23, 2015

@doutriaux1 can we delete other modules?

aashish24 added a commit that referenced this pull request Jun 23, 2015

Merge pull request #1394 from UV-CDAT/flake8
Change pep8 to flake8.

@aashish24 aashish24 merged commit 3b155ac into master Jun 23, 2015

3 of 4 checks passed

continuous-integration/kitware-buildbot/uvcdat-test-laptop-linux-release/ Build done.
Details
continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aashish24 aashish24 deleted the flake8 branch Jun 23, 2015

@durack1

This comment has been minimized.

Member

durack1 commented Jun 23, 2015

@dlonie @aashish24 the problem with fetching resources (flake8 above) and connecting to the dap server (issues dealing with timeouts during cdms opendap tests (see here, here and here)) seems to indicate a flaky network connection on the test-laptop - is there a way to make this connection more robust? Or potentially trigger CMake to re-try failed downloads a number of times..?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 23, 2015

@aashish24 will address (someday) the remval of other modules as part of @durack1 PR #1341

@durack1

This comment has been minimized.

Member

durack1 commented Jun 23, 2015

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

Skipping cdtime, as it just has C code.

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

psql too.

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 23, 2015

All test PRs are in. I've edited my comment above to contain links to each module's PR.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 23, 2015

Thanks @dlonie ! Let the fun begin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment