search

pusher / pusher-http-ruby

Ruby library for Pusher Channels HTTP API
https://pusher.com/channels
MIT License
664 stars 123 forks source link

Declare em-http-request as a runtime dep #119

Closed wjessop closed 6 years ago

wjessop commented 7 years ago

Pusher require em-http-request:

https://github.com/pusher/pusher-http-ruby/blob/adf04980682264050e70b99dd39dc8b82af2390a/lib/pusher/client.rb#L377

but it is only specified as a development dependency meaning it is not installed when a bundle install is done in a project that includes it. This means em-http-request has to be explicitly added to the Gemfile of a project.

This PR changes it to a regular runtime dependency so Bundler knows about it and can install the correct version as required.

wjessop commented 7 years ago

OK, I've just seen that it seems that it is deliberate that em-http-request is not included in as a runtime dependency deliberately. There's two things that may be an issue here:

  1. Developers are getting the error: LoadError (cannot load such file -- em-http) when em-http-request is not added to the Gemfile.
  2. You specify a version of em-http-request ("~> 1.1.0") as a development dependency, but there's no automatic version resolution for non-development. eg. if the version of em-http-request the pusher gem requires is bumped then that is not automatically handled by bundler in it's dependency resolution.

Any thoughts on the above?

wjessop commented 7 years ago

OK, possible plan. A new Gem owned by pusher called "pusher-http-ruby-em" that does nothing but depend on both pusher-http-ruby, and em-http-request. That way people who want to use EM can depend on that one gem and get their deps resolved properly by bundler, and pusher can control bumping the version and dependent versions of em-http-request.

jameshfisher commented 6 years ago

Thanks! As you noted, this gem does not include em-http-request as a dependency, even though it uses it at runtime in some contexts. I believe this is by design, as not everyone is using EventMachine. There are some trade-offs, such as the required em-http-request version not being managed - we could fix this by documenting supported versions of em-http-request in the README.