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

Partition this gem? #23

Closed
dominicsayers opened this issue Nov 12, 2015 · 8 comments
Closed

Partition this gem? #23

dominicsayers opened this issue Nov 12, 2015 · 8 comments

Comments

@dominicsayers
Copy link

Have you considered partitioning this gem into (maybe) slack_web_client and slack_real_time_client?

For my own project I have no need of the real time client, and it doesn't look like the web client depends on it at all. This would reduce the amount of code in my project and remove the dependency on faye-websocket too.

The real time client gem could then be built with the web client as a dependency.

@dblock
Copy link
Collaborator

dblock commented Nov 12, 2015

I am not against it, but that wasn't built separately from the start, so here we are.

I think Slack will continue introducing a lot of other API things and it's easier to have a single client deal with everything. Regarding dependencies we want the EM dependency to be removed anyway unless you use EventMachine, see https://github.com/dblock/slack-ruby-client/issues/5, so it will achieve that part for you.

@dblock
Copy link
Collaborator

dblock commented Nov 12, 2015

Btw, @dominicsayers you could help by completing part of #5 which extracts and abstracts the EM code without adding the Celluloid version. That alone would be a valuable addition and would solve your request on removing the faye-websocket dependency.

@dblock
Copy link
Collaborator

dblock commented Nov 13, 2015

Celluloid support has been merged on HEAD, which removes the hard dependency on eventmachine and faye-websocket, @dominicsayers please give it a try.

@dominicsayers
Copy link
Author

Will do, thanks @dblock

@dominicsayers
Copy link
Author

Working fine for me. About to release my project to staging without faye-websocket

@dblock
Copy link
Collaborator

dblock commented Nov 17, 2015

I will close this as "won't fix" since I don't like the idea of splitting the gem. Thanks for your suggestion!

@dblock dblock closed this as completed Nov 17, 2015
@pusewicz
Copy link

pusewicz commented Aug 6, 2019

@dblock For people who do not use the realtime functionality, many pulled in gems are not necessary and they are a liability and/or maintenance cost. The websocket-driver is a good candidate to get rid of, so is gli.

@dblock
Copy link
Collaborator

dblock commented Aug 7, 2019

We discussed this later in #229 for web/rtm api split and talked about splitting the command line in #128. I think the later makes immediate sense, @pusewicz feel free to do the work and we can delete the gli dependency from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants