Support setting the OpsGenie alert priority on a per-check basis. #27
Conversation
|
@Castaglia that seems like a good idea to me, it does raise some questions that I think we should discuss in #28 prior to code review. I look forward to hearing your thoughts especially as a user of opsgenie. |
|
Any additional work/thoughts on this PR? |
|
Sorry will review, easy to lose track of all of these and never enough time. |
bin/handler-opsgenie.rb
Outdated
| @@ -118,6 +120,19 @@ def create_alert | |||
| teams: json_config['teams']) | |||
| end | |||
|
|
|||
| def event_priority | |||
| return nil unless json_config['priority'] | |||
There was a problem hiding this comment.
I'd suggest returning false or DEFAULT_PRIORITY rather than returning nil is often harder to trace if it is a bug or intended.
There was a problem hiding this comment.
Excellent point. PR updated.
|
@Castaglia checking if you have time to circle back. I see that you will need to rebase to fix conflicts, let me know if you need help with that. |
…he code reads more consistently with other similar use cases.
Castaglia
force-pushed
the
per-check-priority
branch
from
84dd393 to
de13759
Compare
Can't argue with you there, thats why they expose config and allow inline disables. That being said rubocop has made me a much better developer both in the sense of learning to follow commonly accepted convention and more importantly learn why the rules exist and know when to disagree from an informed stance. If you are talking about long running applications ya a long block is probably a bad idea but in a short lived monitoring script there is less need to worry about such things. |
Pull Request Checklist
Is this in reference to an existing issue?
General
Update Changelog following the conventions laid out here
Update README with any necessary configuration snippets
Binstubs are created if needed
RuboCop passes
Existing tests pass
New Plugins
Tests
Add the plugin to the README
Does it have a complete header as outlined here
Purpose
Known Compatibility Issues