-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore(source): reduce cyclomatic complexity of extractNodePortTargets #5444
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
chore(source): reduce cyclomatic complexity of extractNodePortTargets #5444
Conversation
Hi @linoleparquet. Thanks for your PR. I'm waiting for a kubernetes-sigs 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-sigs/prow repository. |
/ok-to-test |
} | ||
|
||
// pods retrieves a slice of pods associated with the given Service | ||
func (sc *serviceSource) pods(svc *v1.Service) []*v1.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.
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.
Yep, wanted to, but I don't see an easy way to test these functions.. Also they are essentially basic calls to the apimachinery and client-go package, without custom logic. I'm not sure how much value unit testing these would add in this case. Happy to hear suggestions if you think there’s a good approach here
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.
No probs. I know how to write tests, but too much to mock ;-). I'll have a look at some point, as I could imagine we may going to have quite few similar methods at some point.
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
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivankatliarchuk, mloiseleur 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 |
Description
Reducing cyclomatic complexity of the extractNodePortTargets function.
Related to #5419
Checklist