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

Add python lint tests. #362

Closed
wants to merge 1 commit into from
Closed

Add python lint tests. #362

wants to merge 1 commit into from

Conversation

nanliu
Copy link

@nanliu nanliu commented Feb 1, 2018

This change is Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@k8s-ci-robot
Copy link

Hi @nanliu. Thanks for your PR.

I'm waiting for a kubernetes or tensorflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@nanliu
Copy link
Author

nanliu commented Feb 1, 2018

Fixes #53

@coveralls
Copy link

coveralls commented Feb 1, 2018

Coverage Status

Coverage remained the same at 31.746% when pulling 6384366 on nanliu:ci into 330eb92 on tensorflow:master.

@jlewi
Copy link
Contributor

jlewi commented Feb 1, 2018

Thanks for the contribution. We already run pylint in prow. And I think we are already running gotomelinter in travis. What is the remaining work to do with fix lint?

@nanliu
Copy link
Author

nanliu commented Feb 1, 2018

Ah, I wasn't aware of that. If it's no longer an issue feel free to close the PR and issue. I was doing something quick to check if my cla is working. Looks like I have to go track down some stuff on my end with our corporate cla.

.travis.yml Outdated
# For now though we just run all tests in pkg.
- $GOPATH/bin/goveralls -service=travis-ci -v -package ./pkg/...
- language: python
python: 2.7
Copy link
Member

Choose a reason for hiding this comment

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

can we please use 3.6?

Copy link
Author

Choose a reason for hiding this comment

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

After toggling it, I just remember the lint results are not very happy in 3.6 with a bunch of deprecation warnings, so I assumed the project was set to 2.7.

@jlewi
Copy link
Contributor

jlewi commented Feb 1, 2018

I took a look at #53 and added some notes about what's missing.

For py it looks like we are missing pep8 check.

If you wanted to add pep8 check you could add it to our existing run_lint function
https://github.com/tensorflow/k8s/blob/master/py/py_checks.py#L18

That way it would be automatically invoked by prow and reported in prow (as opposed to using travis).

markdown autoformatting would be super nice as well. I think I'm seeing a lot of spurious changes in markdown PRs because everyone formats files differently. So would be really nice to start defining some standard conventions to cut this down.

@nanliu
Copy link
Author

nanliu commented Feb 6, 2018

I signed it!

@jlewi jlewi closed this Feb 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants