-
-
Notifications
You must be signed in to change notification settings - Fork 38
add flake8 to travis testing #51
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AZtheAsian welcome! Thanks for taking this, you are almost there, please read my inline comment.
.travis.yml
Outdated
- pip install -r requirements-test.txt | ||
|
||
# command to run flake8 | ||
before_script: "flake8 --ignore=E501,W291,W293 netdiff" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs improvement, please proceed as follows:
- set the line length to 110 characters in a similar fashion this example setup.cfg file (it means adding a flake8 section and a
max-line-length = 110
line - remove the
--ignore
flag entirely - fix any issue that comes up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AZtheAsian Also squash all the commits into and change the commit message to something like [QA] Added flake8 check to Travis tests
. QA means Quality Assurance and also notice that we've used past tense in the commit messages. Good job btw!
universal=1 | ||
|
||
[flake8] | ||
max-line-length = 110 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need a newline at line number 5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AZtheAsian sorry for the wait, @auvipy is right regarding the new line comment, please pay attention to comments from other mentors too next time, for this time I'll take care of adding the newline.
This ensures that we test for pep8 compliance during Travis instead of self-enforcing.