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

[RFC] Unify handler-slack.rb and handler-slack-multichannel.rb? #128

Closed
kali-brandwatch opened this issue Apr 11, 2019 · 3 comments
Closed

Comments

@kali-brandwatch
Copy link

On my PR for handler-slack-multichannel.rb (see here) I modified the channel composition from the original created by Rob Wilson in his initial commit (99e1baa9) so that the handler accepts both arrays of channels and strings (single channel), which is the standard in the handler-slack.rb.

While discussing about the PR I realized that if we port this support for both strings and arrays to the handler-slack.rb it would be no longer necessary to keep 2 versions of the handler, which have already diverged in some ways (notable example: handling of non-standard status codes in the message composition, which was breaking the execution), making it difficult to maintain both properly.

It also makes it hard to justify to users to have both versions set in their system asking them to use one or the other depending on the scenario.

I think it would be easier to just port this (should be easy) and completely remove handler-slack-multichannel.rb.

@majormoses
Copy link
Member

majormoses commented Apr 12, 2019

This could be moved over to the slack specific repository, the other RFC made sense in this repo as we are discussing it across all plugins where RFC is specific to slack plugin. The short answer: that sounds like a good idea to me. Please open an issue on the correct repository so we can track the progress

In order to do that we need to:

  1. a deprecation warning to let admins know the change is coming (prs are welcome)
  2. do a minor release (maintainer)
  3. document the differences and call them out in the changelog so people know how to upgrade (prs are welcome)
  4. do a major release (maintainer)

As people should be pinning their versions people should be able to continue to function with the old one for as long as they want.

@kali-brandwatch
Copy link
Author

Sorry I mistakenly open this in the wrong place. Will do where it belongs.
Thanks for your patience with the newbie!

@majormoses
Copy link
Member

No worries, I am happy to help. Also if you have questions feel free to hit me up in the community slack. I may not respond right away but I do respond to everything I get.

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

No branches or pull requests

2 participants