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

fixed some golint warning #486

Merged
merged 4 commits into from
Mar 23, 2018
Merged

Conversation

kayush2O6
Copy link
Contributor

@kayush2O6 kayush2O6 commented Mar 21, 2018

I have deleted my old PR since too many merge conflicts were occurring while rebasing the branch.
I have fixed some warnings as addressed by the issue. I have generated PR just to make sure that nothing is broken till now.
I have followed the convention as suggested by @jlewi .


This change is Reviewable

@gaocegege
Copy link
Member

/ok-to-test

Thanks for your awesome contribution! Please fix the errors in travis build

@@ -24,7 +24,7 @@ func addDefaultingFuncs(scheme *runtime.Scheme) error {
}

// SetDefaultsTFJob initializes any uninitialized values to default values
func SetDefaults_TFJob(obj *TFJob) {
func SetDefaultsTFJob(obj *TFJob) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not update the code here sine it is the convention for k8s defaulter-gen.

Copy link
Member

Choose a reason for hiding this comment

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

I think we could revert the change here and set the file as ignored files in linter_config.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay! sure I will revert the last commit and add that file in linter_config.json

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Sorry for the late reply, or we could avoid the problem.

Thanks for your contribution, again.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 45.25% when pulling 015e90b on AK-ayush:enhancement-394 into 77bcdc4 on kubeflow:master.

@coveralls
Copy link

coveralls commented Mar 21, 2018

Coverage Status

Coverage increased (+0.2%) to 45.418% when pulling a40dd6a on AK-ayush:enhancement-394 into 77bcdc4 on kubeflow:master.

@gaocegege
Copy link
Member

/lgtm

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit c7e3940 into kubeflow:master Mar 23, 2018
gaocegege added a commit that referenced this pull request Mar 23, 2018
Signed-off-by: Ce Gao <gaoce@caicloud.io>
jetmuffin pushed a commit to jetmuffin/tf-operator that referenced this pull request Jul 9, 2018
* fixed some golint warning

* fixed some golint warning

* updated linter_config.json
jetmuffin pushed a commit to jetmuffin/tf-operator that referenced this pull request Jul 9, 2018
Signed-off-by: Ce Gao <gaoce@caicloud.io>
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.

4 participants