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

Flake8 #30

Merged
merged 7 commits into from Aug 15, 2017
Merged

Flake8 #30

merged 7 commits into from Aug 15, 2017

Conversation

@loicgasser
Copy link
Contributor

@loicgasser loicgasser commented Aug 10, 2017

Fix for #29

I using the default flake8 command to fix the errors.

In the tests I am ignoring line length, as we can have some long mapfile strings.

I fixed most of the errors using autopep8.

A useful command:
autopep8 --in-place --aggressive --aggressive --verbose $(find mappyfile/* -type f -name "*.py" -print)

@loicgasser
Copy link
Contributor Author

@loicgasser loicgasser commented Aug 10, 2017

I just added a coverage report.

Name                       Stmts   Miss  Cover
----------------------------------------------
mappyfile/__init__.py          3      0   100%
mappyfile/ordereddict.py      31     13    58%
mappyfile/parser.py           68      8    88%
mappyfile/pprint.py          173     15    91%
mappyfile/tokens.py            4      0   100%
mappyfile/transformer.py     162     14    91%
mappyfile/utils.py            42      5    88%
mappyfile/validator.py        20     14    30%
----------------------------------------------
TOTAL                        503     69    86%

even though the main issue is that you won't know how grammar you're actually covering, that's another story.

@loicgasser loicgasser force-pushed the loicgasser:flake8 branch 2 times, most recently from e31deec to 7687561 Aug 10, 2017
@loicgasser loicgasser force-pushed the loicgasser:flake8 branch from 7687561 to 21f7b46 Aug 10, 2017
@geographika
Copy link
Owner

@geographika geographika commented Aug 12, 2017

Many thanks for this. I'll review properly next week when I'm back at work.

@geographika geographika merged commit 23b6f87 into geographika:master Aug 15, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geographika
Copy link
Owner

@geographika geographika commented Aug 15, 2017

@loicgasser - that's great to have setup. Are you ok with being added to a "Contributers" section in the docs?

@loicgasser
Copy link
Contributor Author

@loicgasser loicgasser commented Aug 16, 2017

sure

@loicgasser loicgasser deleted the loicgasser:flake8 branch Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.