-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
PushoverAgent: Treat parameter options as templates rather than default values #1720
Conversation
…lt values This should fix #1718. While at it, character limit implementation is fixed: `slice(0..n)` would take the first n+1 characters instead of n.
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.
Very cool usage of capture
and default
in the migration!
end | ||
|
||
def suffix_for(key) | ||
"{% endcapture %}" << DEFAULT_OPTIONS[key].sub(/(?=\}\}\z)/, '| default: _default_ ') |
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.
Shuldn't that be '| default: _default_ }}'
?
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.
Yea, I think it needs the }}
?
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.
Actually (?=\}\}\z)
is a zero-width pattern so I don't need to overwrite the }}
in the match. 😁
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.
Interesting, one can learn something new about regex every day 😛
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.
Nice :)
* `token`: your application's API token | ||
* `user`: the user or group key (not e-mail address). | ||
* `expected_receive_period_in_days`: is maximum number of days that you would expect to pass between events being received by this agent. | ||
|
||
Your event can provide any of the following optional parameters or you can provide defaults: | ||
The following options are all Liquid templates which evaluated values will be posted to Pushover API. Only the `message` parameter is required, and if it is blank API call is omitted. |
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.
"The following options are all Liquid templates whose evaluated values will be posted to the Pushover API."
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.
Thanks!
Very interesting! |
Thanks for the review and testing, I'm merging this! |
This should fix #1718.
While at it, character limit implementation is fixed:
slice(0..n)
would take the first n+1 characters instead of n.