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

Wrap slack_icon with colons #495

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Wrap slack_icon with colons #495

merged 1 commit into from
Jun 18, 2019

Conversation

0x6d617474
Copy link
Contributor

Pull Request (PR) description

The configuration file for the webhook config writes values that
begin with colons (:) as symbols rather than strings, to allow
for calculated values.

Since slack emoji names follow the pattern of :name:, they are
written as symbols instead of strings, and the resulting config
file breaks the webhook service.

This change simply requires slack icons to be entered without the
wrapping colons, and adds them back just before sending to Slack.

Where previously the ocean icon would be written as :ocean: in
the webhook_slack_icon parameter, now it would be written as ocean.

This Pull Request (PR) fixes the following issues

n/a

The configuration file for the webhook config writes values that
begin with colons (:) as symbols rather than strings, to allow
for calculated values.

Since slack emoji names follow the pattern of `:name:`, they are
written as symbols instead of strings, and the resulting config
file breaks the webhook service.

This change simply requires slack icons to be entered without the
wrapping colons, and adds them back just before sending to Slack.

Where previously the ocean icon would be written as `:ocean:` in
the webhook_slack_icon parameter, now it would be written as `ocean`.
@0x6d617474 0x6d617474 mentioned this pull request Jun 17, 2019
@rnelson0
Copy link
Member

Does this deserve a little disclaimer somewhere in the docs, so people know to adjust?

@0x6d617474
Copy link
Contributor Author

I don't think the slack_icon feature is mentioned anywhere in the documentation, and if you tried to use it before this patch you'd get a broken service, so I don't think this change will affect anyone negatively.

I can use this PR to update the docs to include the slack_icon feature if you'd like.

@rnelson0 rnelson0 merged commit 16c4aa8 into voxpupuli:master Jun 18, 2019
@rnelson0
Copy link
Member

Since it was not documented already, I went ahead and merged it prior to 7.0.0, but please do open a documentation PR when you get a chance!

@0x6d617474 0x6d617474 deleted the slack_icon_fix branch June 18, 2019 21:22
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.

3 participants