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

Fix golint error #45

Merged
merged 1 commit into from Feb 4, 2019
Merged

Fix golint error #45

merged 1 commit into from Feb 4, 2019

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented Jan 23, 2019

Currently golint (./hack/verify-golint.sh ) fails due to following error:

$ ./hack/verify-golint.sh 
!!! golint detected the following problems:
./pkg/controller/csidriverdeployment/objects_test.go:19:25: should omit type corev1.Container from declaration of var defaultDriverContainer; it will be inferred from the right-hand side
./pkg/controller/csidriverdeployment/objects_test.go:26:12: should omit type v1alpha1.CSIDriverDeployment from declaration of var defaultCR; it will be inferred from the right-hand side

This patch fixes the error.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 23, 2019
@jsafrane
Copy link
Contributor

go fmt changes between 1.10 and 1.11. Last time I checked we used 1.10, so anything you push should be formatted with 1.10.

Currently `make verify` fails due to gofmt error. This patch fixes the format.
@nak3
Copy link
Contributor Author

nak3 commented Jan 23, 2019

@jsafrane Thank you! updated (and it passed the Jenkins test.)

@nak3
Copy link
Contributor Author

nak3 commented Jan 23, 2019

Ah, I misunderstood. The format error fixed in this PR causes error with go1.11, but go 1.10 misses this error. So, verify test passes either with or without this on go1.10.

@nak3 nak3 closed this Jan 24, 2019
@nak3 nak3 deleted the fix-gofmt-error branch January 24, 2019 12:52
@nak3 nak3 restored the fix-gofmt-error branch January 25, 2019 13:28
@nak3 nak3 changed the title Fix gofmt error Fix golint error Jan 25, 2019
@nak3 nak3 reopened this Jan 25, 2019
@nak3
Copy link
Contributor Author

nak3 commented Jan 25, 2019

@jsafrane I'm sorry for bothering you. But I realized that this is an issue of golint and whenever I run make verify or hack/verify-golint.sh, the script always gets error.

$ ./hack/verify-golint.sh 
!!! golint detected the following problems:
./pkg/controller/csidriverdeployment/objects_test.go:19:25: should omit type corev1.Container from declaration of var defaultDriverContainer; it will be inferred from the right-hand side
./pkg/controller/csidriverdeployment/objects_test.go:26:12: should omit type v1alpha1.CSIDriverDeployment from declaration of var defaultCR; it will be inferred from the right-hand side

Can I ask you what golint version do you use? I tested on my older and newer Fedora (golint-0.0.x) but both fails.

@jsafrane
Copy link
Contributor

jsafrane commented Feb 4, 2019

Interesting, I use golint directly from github (go get -u golang.org/x/lint/golint). It does not understand any --version parameter :-(. I wonder what golang-1.10 image in ci/prow/verify uses.

Anyway, this looks harmless.

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 4, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, nak3

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 4, 2019
@openshift-merge-robot openshift-merge-robot merged commit 56ed279 into openshift:master Feb 4, 2019
@nak3 nak3 deleted the fix-gofmt-error branch February 4, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants