Skip to content

fix: uniqMcs use all cpu#246

Open
ningmingxiao wants to merge 1 commit intoopencontainers:mainfrom
ningmingxiao:dev03
Open

fix: uniqMcs use all cpu#246
ningmingxiao wants to merge 1 commit intoopencontainers:mainfrom
ningmingxiao:dev03

Conversation

@ningmingxiao
Copy link
Copy Markdown
Contributor

No description provided.

Comment on lines +87 to +88

func GetContainerLabelsSize() int {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func GetContainerLabelsSize() int {
func ContainerLabelsSize() int {

Per Go coding guidelines, Get should not be used (see https://google.github.io/styleguide/go/decisions#getters).

I know we have GetEnabled but it's the only function.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I also don't like the fact that we introduce a new public function which is not really used anywhere except in our own code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have to introduce a new public function.

@ningmingxiao ningmingxiao force-pushed the dev03 branch 2 times, most recently from d083957 to 4567cb2 Compare March 17, 2026 02:01
@rhatdan
Copy link
Copy Markdown
Collaborator

rhatdan commented Mar 23, 2026

LGTM

Copy link
Copy Markdown
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

Maybe we need to change the public API to add error reporting to, say, selinux.ContainerLabels (and similar functions using addMcs/uniqMcs)?
Keep the old functions for backward compatibility but deprecate those?

@ningmingxiao
Copy link
Copy Markdown
Contributor Author

Maybe we need to change the public API to add error reporting to, say, selinux.ContainerLabels (and similar functions using addMcs/uniqMcs)? Keep the old functions for backward compatibility but deprecate those?

I afraid user have to adapt to the new interface and we can't let user know old interface is deprecate. @kolyshkin

Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
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