-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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 failures of e2e/framework/util.go - part2 #76488
Fix golint failures of e2e/framework/util.go - part2 #76488
Conversation
Hi @atoato88. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
/ok-to-test |
/priority backlog |
/lgtm |
test/e2e/framework/util.go
Outdated
@@ -1515,7 +1516,7 @@ func podRunning(c clientset.Interface, podName, namespace string) wait.Condition | |||
} | |||
} | |||
|
|||
// WaitTimeoutForPodEvent waits for an event to occur for a pod | |||
// WaitTimeoutForPodEvent waits for an event to occur for a pod. |
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.
added some minor comments.
// WaitTimeoutForPodEvent waits for an event to occur for a pod. | |
// WaitTimeoutForPodEvent waits the given timeout duration for a pod event to occur. |
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.
Thank you for comment.
I've fixed this.
test/e2e/framework/util.go
Outdated
@@ -1856,13 +1859,14 @@ type podProxyResponseChecker struct { | |||
pods *v1.PodList | |||
} | |||
|
|||
func PodProxyResponseChecker(c clientset.Interface, ns string, label labels.Selector, controllerName string, respondName bool, pods *v1.PodList) podProxyResponseChecker { | |||
return podProxyResponseChecker{c, ns, label, controllerName, respondName, pods} | |||
// GetPodProxyResponseChecker returns a context for checking a pods responses. |
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.
Is there a grammatical problem here?
// GetPodProxyResponseChecker returns a context for checking a pods responses. | |
// GetPodProxyResponseChecker returns a context for checking pods responses. |
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.
done.
test/e2e/framework/util.go
Outdated
@@ -2098,6 +2109,7 @@ func ServiceResponding(c clientset.Interface, ns, name string) error { | |||
}) | |||
} | |||
|
|||
// RestclientConfig returns a config holds the information needed to build connect to kubernetes clusters. |
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.
// RestclientConfig returns a config holds the information needed to build connect to kubernetes clusters. | |
// RestclientConfig returns a config holds the information needed to build connection to kubernetes clusters. |
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.
done.
1eee2d6
to
c2346c9
Compare
test/e2e/framework/util.go
Outdated
func PodProxyResponseChecker(c clientset.Interface, ns string, label labels.Selector, controllerName string, respondName bool, pods *v1.PodList) podProxyResponseChecker { | ||
return podProxyResponseChecker{c, ns, label, controllerName, respondName, pods} | ||
// GetPodProxyResponseChecker returns a context for checking pods responses. | ||
func GetPodProxyResponseChecker(c clientset.Interface, ns string, label labels.Selector, controllerName string, respondName bool, pods *v1.PodList) PodProxyResponseChecker { |
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.
Is prepending this method with Get
something that golint enforces?
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.
Looks like this is a constructor method, what are your thoughts on NewPodProxyResponseChecker
instead?
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 renaming seems for avoiding duplicated names between struct name (PodProxyResponseChecker which is returned with this method) and this method name.
NewPodProxyResponseChecker
is also a good option for us I feel.
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.
done.
test/e2e/framework/util.go
Outdated
func NewKubectlCommand(args ...string) *kubectlBuilder { | ||
b := new(kubectlBuilder) | ||
// GetNewKubectlCommand returns a KubectlBuilder for running kubectl. | ||
func GetNewKubectlCommand(args ...string) *KubectlBuilder { |
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.
I liked NewKubectlCommand
actually, was golint complaining about this?
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.
Thank you for comment.
I've fixed this.
c2346c9
to
0a56fe4
Compare
I rebased with current master(commit: b359b6b). |
This is a part of a series for fixing golint failures for util.go. - fixes golint failures from line 1395 to line 2353 at original util.go This fixes golint failures of the following file: - test/e2e/framework/util.go This changes following files because of change function name in above file. - test/e2e/apps/rc.go - test/e2e/apps/replica_set.go
0a56fe4
to
85f21c1
Compare
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.
Thanks for updating
/lgtm
/approve |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atoato88, oomichi, timothysc 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This is a part of a series for fixing golint failures for util.go.
This fixes golint failures of the following file:
This changes following files because of change function name
in above file.
Special notes for your reviewer:
The others of this series are following:
Does this PR introduce a user-facing change?:
Ref: #68026