Pingdom integration #537
Pingdom integration #537
Conversation
|
Automated message from Dropbox CLA bot @TomaszKolek, it looks like you've already signed the Dropbox CLA. Thanks! |
| @@ -1179,6 +1185,31 @@ | |||
|
|
|||
| </div> | |||
|
|
|||
| <div id="pingdom" class="integration-instructions"> | |||
|
|
|||
| <p>Zulip supports Pingdom integration and can notify you of | |||
There was a problem hiding this comment.
The English here could be a little cleaner, e.g. "Zulip supports integration with Pingdom". Might as well fix the similar pattenr in the Phabricator integration just above it.
There was a problem hiding this comment.
For me is understandable but maybe is Polish-English ;-)
Changed in 3 places
|
This looks pretty good, thanks for working on this @TomaszKolek ! I posted a bunch of small comments. |
TomaszKolek
force-pushed
the
pingdoms-integration
branch
from
6ab2100
to
d75584d
Compare
|
Oh, looks like you posted a new version @TomaszKolek! Just FYI, you should post a comment when you do that, because GitHub (sadly) won't sent an email notification when a PR is updated, so I didn't know you had worked on this further. |
| @@ -8,8 +8,8 @@ | |||
| import ujson | |||
|
|
|||
|
|
|||
| PINGDOM_SUBJECT_TEMPLATE = 'Your Pingdom {name} descries something important.' | |||
| PINGDOM_MESSAGE_TEMPLATE = 'Service {service_url} changed it\'s status from {previous_state} to {current_state}.' | |||
| PINGDOM_SUBJECT_TEMPLATE = '{name}: {type} - from {previous_state} to {current_state}.' | |||
There was a problem hiding this comment.
I think the subject should be just {name} status. The reason is that it's actually a lot more useful for this sort of alerting integration if the UP=>DOWN change gets threaded with the same subject as the DOWN=>UP change.
|
Cool, posted one comment on the subject format; you should also squash the commit together to just be a single "Add pingdom integration." commit, but otherwise this looks good to me! |
TomaszKolek
force-pushed
the
pingdoms-integration
branch
from
d75584d
to
202d161
Compare
|
Hm... I don't know what I actually did... (Probably rebase with zulip:master) @timabbott Do you know how can I fix it? In worst case I will copy my content into separated branch |
|
You should be able to fix this with |
TomaszKolek
force-pushed
the
pingdoms-integration
branch
3 times, most recently
from
828877b
to
2032bbe
Compare
|
@timabbott Thanks. |
|
Oh, one more thing -- usually our integration docs have a screenshot of an example message produced by the integration to show what it looks like -- can you add one to the documentation? |
|
@TomaszKolek just wanted to follow up on this since it seems like you're nearly done... |
| <img class="screenshot" src="/static/images/integrations/pingdom/001.png" /> | ||
|
|
||
| <p>Last, during creating or editing your check, scroll down to <code>Connect Integrations</code> | ||
| section and ensure your integration is checked </p> |
There was a problem hiding this comment.
Needs a period at the end of this sentence?
TomaszKolek
force-pushed
the
pingdoms-integration
branch
2 times, most recently
from
60f3a9f
to
ac5a309
Compare
TomaszKolek
force-pushed
the
pingdoms-integration
branch
from
ac5a309
to
52afff1
Compare
|
@timabbott |
|
Merged, thanks @TomaszKolek for the contribution! |
Implementation
Unit tests
Integrations documentation