First-pass at improving template system. #147
Conversation
brian-brazil
force-pushed
the
new-email
branch
from
e8c3369 to
4a00998
Compare
| ) | ||
|
|
||
| var ( | ||
| // DefaultHipchatConfig defines default values for Hipchat configurations. | ||
| DefaultHipchatConfig = HipchatConfig{ | ||
| ColorFiring: "purple", | ||
| ColorResolved: "green", | ||
| Color: `{{ if eq .Status "firing" }}purple{{ else }}green{{ end }}`, |
There was a problem hiding this comment.
This is the mix of template files and inlined templating I'm not yet sure about.
Then again, this is one of a bunch of configs taken from the old implementation that are not backed by an implementation.
Maybe it makes sense to remove these as well to start unbiased by the old structure when adding new integrations.
There was a problem hiding this comment.
I'm okay with kicking this one down to the default.tmpl. For things this brief I was thinking it may make sense to do it inline.
There's a few other equivalents that can be replaced by an if in templates, mostly color related.
There was a problem hiding this comment.
Actually fine with that. This was just the first one I saw. But it's consistent what is happening in other places below 👍
|
Needs rebase. |
brian-brazil
force-pushed
the
new-email
branch
from
4a00998 to
e1da3e9
Compare
I've already done so, I suspect you accidentally |
- Cut back to bare minimum to make the rest simpler - Consistency in config naming - Have one data strucutre that's the same for all templates - Pass in common labels to templates - Support templates almost everywhere - Support multiple SMTP recipients - Support non-ASCII SMTP headers - Handle colour logic via templates - Make $subjects have consistent output, go maps aren't sorted. - Make tests pass when v6 is disabled
brian-brazil
force-pushed
the
new-email
branch
from
e1da3e9 to
faa8883
Compare
| Status string | ||
| Alerts []TemplateAlert | ||
| AlertCommonLabels map[string]string | ||
|
|
There was a problem hiding this comment.
You don't see any use for common annotations? Both suffer from the problem of changing as more alerts are sorted into the group. But if for example summary is common across all of them, that can be useful for some templates.
There was a problem hiding this comment.
I don't think the use case is as strong as for labels, I can see potential use but only in a similar way to what I'm doing to subject which won't tend to have space to fit a summary. I figure a wait and see approach is best, we can always add it in later.
|
Okay, let's merge so we have the base in place 🚢 |
First-pass at improving template system.
@fabxc
Fixes #135 #126 #74
Still needs improvement, but this should be a good base.