-
Notifications
You must be signed in to change notification settings - Fork 132
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
feat: add labels from nri-winservices config as tags on UI. #1989
base: master
Are you sure you want to change the base?
feat: add labels from nri-winservices config as tags on UI. #1989
Conversation
Pull Request Test Coverage Report for Build 12705852369Details
💛 - Coveralls |
entities = append(entities, r.Data.Entity) | ||
entity := r.Data.Entity | ||
// Add labels to Metadata | ||
if r.Definition.Labels != nil && len(r.Definition.Labels) > 0 { |
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.
Is it required to check the nilness
? If Labels
is nil
, len(r.Definition.Labels)
will be 0, won't it?
entities = append(entities, r.Data.Entity) | ||
entity := r.Data.Entity | ||
// Add labels to Metadata | ||
if r.Definition.Labels != nil && len(r.Definition.Labels) > 0 { |
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.
Does it make sense to extract it in a short util func and add some unit tests? (Would improve coverage as well :))
But it's totally fine if we don't do this.
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.
Adding the unit tests for this is an excellent point ;-)
Add labels from
nri-winservices
config as tags on the UI.