Skip to content
This repository has been archived by the owner on Sep 24, 2021. It is now read-only.

Avoid panic when labels are nil #118

Merged
merged 1 commit into from
Apr 1, 2017

Conversation

xlgao-zju
Copy link
Contributor

Frakti will panic, when we annotation is not nil but labels are nil.

/cc @feiskyer

Signed-off-by: Xianglin Gao [email protected]

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 1, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@@ -187,13 +187,18 @@ func buildLabelsWithAnnotations(labels, annotations map[string]string) map[strin
return labels
}

newLables := make(map[string]string)
if labels != nil {
newLables = labels
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it's better to exchange the position between newLables = labels and make(map[string]string)

Copy link
Contributor Author

@xlgao-zju xlgao-zju Apr 1, 2017

Choose a reason for hiding this comment

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

Yes, will do. make(map[string]string) will take more resource than newLables = labels. So newLables = labels is better. Am I right? @feiskyer

@xlgao-zju
Copy link
Contributor Author

@feiskyer Updated

@feiskyer
Copy link
Contributor

feiskyer commented Apr 1, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2017
@feiskyer feiskyer merged commit bc83cd6 into kubernetes-retired:master Apr 1, 2017
@xlgao-zju xlgao-zju deleted the fix-nil-labels branch April 1, 2017 15:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants