Skip to content

Conversation

@jimexist
Copy link
Member

@jimexist jimexist commented Dec 30, 2017

fixes #256

No good way to tell whether a file was generated. a better way is to ensure that all files and newly submitted ones are goimported. also from now on I think it is better to enforce that even generated code are pre-goimported.

I didn't fix all the lint warnings because I am afraid of changing any code other than using goimport. I suggest that the original author do the changes instead.


This change is Reviewable

@k8s-ci-robot
Copy link

Hi @jimexist. 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.

@coveralls
Copy link

coveralls commented Dec 30, 2017

Coverage Status

Coverage remained the same at 28.518% when pulling 54cdeff on Jimexist:fix-256 into ed7cae7 on tensorflow:master.

@coveralls
Copy link

coveralls commented Dec 30, 2017

Coverage Status

Coverage decreased (-0.06%) to 28.458% when pulling 04eef00 on Jimexist:fix-256 into ed7cae7 on tensorflow:master.

@zjj2wry
Copy link
Member

zjj2wry commented Dec 30, 2017

/ok-to-test

"LineLength": 240,
"Exclude": ["comment or be unexported", "comment on exported"],
"Exclude": [
"redundant return statement",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think redundant return is to because that's a lint issue in one of the auto copy files. Instead of excluding this warning for all files can we just exclude the deepcopy file from lint checks?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok i will try to do a follow up PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I went ahead and submit this because it fixes the travis build

@jlewi
Copy link
Contributor

jlewi commented Jan 3, 2018

Going to merge this so we fix the build.

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.

Build failing because of lint issues

5 participants