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

Tox, travis and requirements #152

Merged
merged 2 commits into from
Jul 25, 2018
Merged

Conversation

bcaller
Copy link
Collaborator

@bcaller bcaller commented Jul 24, 2018

Requirements:

Requirements files aren't used. Requirements.txt and setup.py had odd
requirements that I can't see used anywhere. Requirements-dev.txt had
conflicting packages (flake8 wants specific versions of pyflakes and
pycodestyle).

Tox & travis:

Split tox into a test, coverage and lint phase. Run either:

tox  # Run all
tox -e py36
tox -e cover
tox -e lint

Tox and travis will now fail the lint / build on flake8 errors to
avoid non-compliant code being merged. (--exit-zero removed)

Coverage will fail for now, so let's not put it in the travis build yet.

McCabe complexity is annoying and dealt with better by codeclimate so I
bumped it up from 10 to 11 so it won't fail at the moment.

@KevinHock KevinHock self-requested a review July 24, 2018 18:24
Copy link
Collaborator

@KevinHock KevinHock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for doing this :) I missed it in my massive 'put everything into their own folders' re-org

.travis.yml Outdated
- coverage run -m tests
- flake8 . --count --exclude=examples --max-complexity=11 --max-line-length=127 --show-source --statistics
- coverage report --include=tests/* --fail-under 100
- coverage report --include=pyt/* --fail-under 70
Copy link
Collaborator

@KevinHock KevinHock Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have somewhere around 90% coverage currently, can you adjust it to whatever it is currently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I was misreading the coverage report I think. Because pyt/vulnerabilities/vulnerability_helper.py has coverage of 76%, but overall it's 90%. I'll fix this tomorrow.

deps = -rrequirements-dev.txt
deps =
coverage>=4.0,<4.4
mock
commands =
coverage erase
coverage run tests
coverage report --include=tests/* --fail-under 100
coverage report --include=pyt/* --fail-under 91
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep it aligned with this as well I suppose.

bcaller added 2 commits July 25, 2018 10:16
In commit

11bcd2d Remove unused function: valid_date

an unused import was left in. With the next commit, tox & travis should
fail:

F401 'datetime.datetime' imported but unused
Requirements:

Requirements files aren't used. Requirements.txt and setup.py had odd
requirements that I can't see used anywhere. Requirements-dev.txt had
conflicting packages (flake8 wants specific versions of pyflakes and
pycodestyle).

Tox & travis:

Split tox into a test, coverage and lint phase. Run either:

tox

tox -e py36

tox -e cover

tox -e lint

Tox and travis will now fail the lint / build on flake8 errors to
avoid non-compliant code being merged. (--exit-zero removed)

Coverage will fail for now, so let's set it really low in travis for
now.

McCabe complexity is annoying and dealt with better by codeclimate so I
bumped it up from 10 to 11 so it won't fail at the moment.
@bcaller
Copy link
Collaborator Author

bcaller commented Jul 25, 2018

I put the code coverage back to 91%. I'd been reading the output incorrectly. 🚢

@KevinHock
Copy link
Collaborator

Awesome :)

@KevinHock KevinHock merged commit 0154caf into python-security:master Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants