Added support more channels #33
Conversation
README.md
Outdated
| set :slack_channel, '#devops' | ||
| set :slack_channel, '#devops' | ||
| # it's possible usage more channel with comma separated | ||
| # set :slack_channel, '#devops,#other-channel' |
There was a problem hiding this comment.
@Scuottolinx thanks for pull request! Could we be more explicit here and accept an array of strings rather than splitting a single string? So something like:
set :slack_channel, ['#devops', '#other-channel']This would mean we would need to make use of Array(fetch(:slack_channel)).each in the cap task itself to ensure that the current set :slack_channel, '#devops' continues to work, but it would avoid any unexpected behaviour from splitting a string such as '#devops, #other-channel'
lib/capistrano/tasks/slackify.cap
Outdated
| fetch(:slack_url) | ||
| fetch(:slack_channel).split(',').each {|channel| | ||
| pay = Slackify::Payload.new(self, :starting) | ||
| pay.channel = (channel) |
There was a problem hiding this comment.
If we're going to pass the channel value into the Payload instance, would it be preferable to pass it as another argument on the initializer? Or alternatively as an argument to build?
Scuottolinx
force-pushed
the
feat/more-channels
branch
from
fccd0be to
0bd1fe4
Compare
|
hi, i have modified |
|
ping please! |
spec/lib/slackify_spec.rb
Outdated
| @@ -41,12 +41,18 @@ module Slackify | |||
|
|
|||
| let(:text) { context.fetch(:slack_text) } | |||
|
|
|||
| let(:builded_payload) { | |||
| Payload.build(context, :success) | |||
| def buildedPayload(channel) | |||
There was a problem hiding this comment.
Can we please revert the test changes here and instead just add the channel variable to Payload.build to match the interface changes? We don't need to iterate over Array(context.fetch(:slack_channel)) here in the test, as we're only passing a single channel to Payload for each iteration.
Scuottolinx
force-pushed
the
feat/more-channels
branch
from
0bd1fe4 to
bfd69be
Compare
|
Great thanks for that, I'll do some testing and if all is well, I'll try to get this merged later on today 👍 |
lib/capistrano/tasks/slackify.cap
Outdated
| fetch(:slack_url) | ||
| Array(fetch(:slack_channel)).each {|channel| | ||
| execute :curl, '-X POST', '-s', '--data-urlencode', | ||
| Slackify::Payload.new(self, :starting, channel), |
There was a problem hiding this comment.
Sorry, one more thing brought up during test - this should be calling build instead of new
Scuottolinx
force-pushed
the
feat/more-channels
branch
from
bfd69be to
e6106f2
Compare
|
@Scuottolinx thanks for that, now available via Rubygems as v2.10.0 |
I have adde the feature requested in the issue #32.
Now it's possible send notification in more channels.