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

Add flake8 test for xmgrace. #1421

Merged
merged 5 commits into from Jul 7, 2015

Conversation

Projects
None yet
3 participants
@allisonvacanti
Contributor

allisonvacanti commented Jun 23, 2015

The test currently fails. The code needs to be cleaned up before merging.

@doutriaux1 @aashish24

doutriaux1 added some commits Jul 6, 2015

finally got flake8 to pass
had to avoid __init__.py
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jul 7, 2015

@aashish24 @dlonie @sankhesh 1 done! Many to go 😉

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jul 7, 2015

You are awesome @doutriaux1 Did you use some tool to fix most of these?

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jul 7, 2015

@dlonie @sankhesh I will have you review it

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented on testing/xmgrace/CMakeLists.txt in 045cd56 Jul 7, 2015

Rather than change this to an explicit list of files to check (which won't catch new added files), use the #noqa comment at the end of a line to ignore specific lines/issues.

../uvcdat/Packages/xmgrace/Lib/__init__.py:1:1: F403 'from xmgrace import *' used; unable to detect undefined names

Or even better, just import the objects that make up this module's public interface explicitly (e.g. rather than using the from module import * syntax in __init__.py, just import the specific bits that need to be exposed). That's how this warning is supposed to be fixed -- import * is considered bad practice in general.

This comment has been minimized.

Contributor

aashish24 replied Jul 7, 2015

yup import * kills the protection 😄 its like free for all...

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jul 7, 2015

The warning should be properly fixed, or the specific line excluded. Let's not change the directory-level scanning.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jul 7, 2015

ok it's in. I left the import * because I don't want to risk breaking anything by forgetting an import. This module is pretty much legacy anyway now, I don't think anybody new to uvcdat would pick this up, it's mainly for our current/past users.

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jul 7, 2015

LGTM

allisonvacanti added a commit that referenced this pull request Jul 7, 2015

Merge pull request #1421 from UV-CDAT/flake8-xmgrace
Add flake8 test for xmgrace.

@allisonvacanti allisonvacanti merged commit 14ec096 into master Jul 7, 2015

3 checks passed

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

@allisonvacanti allisonvacanti deleted the flake8-xmgrace branch Jul 7, 2015

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