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

feat(pkger): add parser support for notification rules #16297

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

jsteenb2
Copy link
Contributor

@jsteenb2 jsteenb2 commented Dec 19, 2019

@jsteenb2 jsteenb2 force-pushed the 4965p/pkger_parser_notif_rules branch from 8d198ed to 5986436 Compare December 19, 2019 20:07
@jsteenb2 jsteenb2 requested a review from a team December 19, 2019 20:07
@jsteenb2 jsteenb2 force-pushed the 4965p/pkger_parser_notif_rules branch from 5986436 to 13fd740 Compare December 19, 2019 20:09
return strings.Title(k.String())
pieces := strings.Split(string(k), "_")
for i := range pieces {
pieces[i] = strings.Title(pieces[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

heh, didn't know this was in strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strings actually has a ton of useful funcs. Has stuff for map, 'trim`ing, and a boat load of other goodies that come up quite often

if c.statusMessage == "" {
vErrs = append(vErrs, validationErr{
Field: fieldCheckStatusMessageTemplate,
Msg: `must provide a template; ex. "Check: ${ r._check_name } is: ${ r._level }"`,
})
}
if status := c.Status(); status != influxdb.Active && status != influxdb.Inactive {
Copy link
Contributor

Choose a reason for hiding this comment

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

demorgans law 👍

}

func (r *notificationRule) Status() influxdb.Status {
if r.status == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

When would status be unset for notification rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question. After speaking with @russorat, we decided that in pkger, you don't have to provide a status, it'll just default to active if not provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, that makes sense

@jsteenb2
Copy link
Contributor Author

tests failing upstream b/c of go vendor issue. This is not related to this PR work. Should not have any affect on any upstream stuffs, no new vendored code here, nor any changes to existing stuffs being used by idpe

@jsteenb2 jsteenb2 merged commit 1d3e0da into master Dec 19, 2019
@jsteenb2 jsteenb2 deleted the 4965p/pkger_parser_notif_rules branch December 19, 2019 20:56
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.

2 participants