search

cloudfoundry-community / slack-notification-resource

Concourse CI resource for sending notifications to Slack.
MIT License
75 stars 82 forks source link

Handle entire Webhook payload generically #9

Closed nebhale closed 7 years ago

nebhale commented 8 years ago

Previously, the implementation of this resource special-cased a small number of the Slack Webhook options that were available. Specifically, this excluded the use of attachments to create richer messages. This change re-implements the out script to allow any properly structured Slack payload. It also ensures that environment variable interpolation occurs in any string in that payload. One of the best side-effects is that the implementation is nearly 60% smaller and is much less complex.

This change also updates the Docker image to be based on alpine, resulting in a drastically smaller image with the same functionality.

r3dDoX commented 7 years ago

I opened an Issue about the same thing some months ago and was notified that you opened this Pull Request. It is the issue: #14. I tried to get your PR mergeable again, but due to the lack of Shell Scripting skills I wasn't able to get it fully working again.

@nebhale Do you have the time to get this PR mergeable again? It would be very nice to have this functionality in the community resource. If you need help with anything I could assist 👍

nebhale commented 7 years ago

@r3dDoX Given that it wasn't merged in the last 8 months, I've given up any hope of getting it merged ever. Instead, I've been using my own fork directly (CI here). I'm not inclined to expend any more effort keeping it up to date with the community resource until there's some indication that it will ever be merged. If someone wants to chime in on this issue with that indication, I'll be happy to fix up the PR to make it ready.

jhunt commented 7 years ago

Sorry @nebhale - didn't see the PR. We'll try to get eyes on this today

analytically commented 7 years ago

Could we have this updated and merged?

jhunt commented 7 years ago

Attachment support was added by tshak 8 days ago, in 03bdec276, @analytically.

I'm sorry that I dropped the ball so hard on this one, @nebhale.

analytically commented 7 years ago

Hi James, attachment was added but is flawed, interpolation doesn't work in the attachment text.

jhunt commented 7 years ago

Can we open a new issue for that then? This PR is unmergable, given the 44 commits that have transpired between branch-cut and now.

analytically commented 7 years ago

https://github.com/cloudfoundry-community/slack-notification-resource/issues/25