Skip to content

plugin: optimize the check for the last registration#5440

Merged
fuweid merged 1 commit intocontainerd:masterfrom
Iceber:optimize-last-registration
May 13, 2021
Merged

plugin: optimize the check for the last registration#5440
fuweid merged 1 commit intocontainerd:masterfrom
Iceber:optimize-last-registration

Conversation

@Iceber
Copy link
Copy Markdown
Member

@Iceber Iceber commented Apr 30, 2021

Optimize the check for the last registration

I originally wanted to change it to:

        var last bool
        for _, requires := range r.Requires {
                if requires == "*" {
                        last = true
                        break
                }
        }
        if last && len(r.Requires) != 1 {
                panic(ErrInvalidRequires)
        }

But I found that the last would not be used elsewhere, so I changed it to:

        for _, requires := range r.Requires {
                if requires == "*" && len(r.Requires) != 1 {
                        panic(ErrInvalidRequires)
                }
        }

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Apr 30, 2021

Hi @Iceber. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 30, 2021

Build succeeded.

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Apr 30, 2021

/ok-to-test

@dmcgowan dmcgowan added this to the 1.6 milestone Apr 30, 2021
fuweid
fuweid approved these changes May 13, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit b9e1d0c into containerd:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzys kzys kzys approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

1.6

Development

Successfully merging this pull request may close these issues.

5 participants

@Iceber @k8s-ci-robot @dmcgowan @kzys @fuweid