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

Not exposed APIReadyCheckFunc to outside of package #1286

Merged

Conversation

s1061123
Copy link
Member

https://github.com/k8snetworkplumbingwg/multus-cni/actions/runs/9225475657 recommends to change the code, so this PR fix the code. In addition, APIReadyCheckFunc can be decaptalize (i.e not expose this type to outisde of package), so change it decaptalized.

comment on exported type APIReadyCheckFunc should be of the form "APIReadyCheckFunc ..." (with optional leading article) test (1.21.x, ubuntu-latest): pkg/server/api/shim.go#L44
type name will be used as api.APIReadyCheckFunc by other packages, and that stutters; consider calling this ReadyCheckFunc

APIReadyCheckFunc is used only in api, hence it can be decapitalize
to make its scope only in this package. This fix changes its scope.
In addition, api.APIReadyCheckFunc seems to be redundant so the name
is changed. Change the comment to fit to golang style, too.
@coveralls
Copy link

Coverage Status

coverage: 63.116%. remained the same
when pulling d23856b on s1061123:fix-type-scope-comment
into 9f5c023 on k8snetworkplumbingwg:master.

// Define a type for API readiness check functions
type APIReadyCheckFunc func(string) error
// readyCheckFunc defines a type for API readiness check functions
type readyCheckFunc func(string) error
Copy link
Member

Choose a reason for hiding this comment

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

can we call it apiReadyCheckFunc

@dougbtv dougbtv merged commit 4757be1 into k8snetworkplumbingwg:master Jun 6, 2024
24 checks passed
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.

3 participants