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

Problematic Sinatra dependency #159

Closed
skalee opened this issue Apr 9, 2017 · 10 comments
Closed

Problematic Sinatra dependency #159

skalee opened this issue Apr 9, 2017 · 10 comments
Labels
status: help wanted requesting help from the community type: bug bug in the library

Comments

@skalee
Copy link
Contributor

skalee commented Apr 9, 2017

Issue Summary

Recently released version 4.1 introduces Sinatra ~> 1.4.7 as a dependency, and that Sinatra version depends on Rack ~> 1.5. In the result, Sendgrid-Ruby is now incompatible with Rails 5 which requires Rack ~> 2.0.

Sinatra 2 works with Rack 2.0. Two release candidates have been pushed to RubyGems already.

Technical details:

  • sendgrid-ruby Version: 4.1, master (latest commit: 59484d9)
  • Ruby Version: any
@thinkingserious thinkingserious added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap type: bug bug in the library and removed type: community enhancement feature request not on Twilio's roadmap labels Apr 10, 2017
@thinkingserious
Copy link
Contributor

Thanks @skalee! I've added this to our backlog for further review.

@awwa, do you have any thoughts on this one?

Reference: http://guides.rubyonrails.org/5_0_release_notes.html (requires ruby 2.2.2+)

@awwa
Copy link
Contributor

awwa commented Apr 19, 2017

Hi @thinkingserious , @skalee,
Sinatra ~> 1.4.7 is not compatible with rails 5. It is better to investigate the compatibility with sinatra 2.0 rc and rails 5 because there might be conflict any other gems. I will check.

@awwa
Copy link
Contributor

awwa commented Apr 19, 2017

@thinkingserious
I checked with ruby 2.3.3 and 2.4.0. It seems no problem. All process was fine. Here is the report.

  1. Update the sendgrid-ruby.gemspec file.
#  spec.add_dependency 'sinatra', '~> 1.4.7'
  spec.add_dependency 'sinatra', '2.0.0.rc2'
  1. Install sinatra 2.0.0.rc2.
bundle install
bundle update
:
Using sinatra 2.0.0.rc2
:
  1. Test sendgrid-ruby
    All tests are fine.
rake test
:
234 runs, 238 assertions, 0 failures, 0 errors, 0 skips
  1. Build package
rake build
sendgrid-ruby 4.2.1 built to pkg/sendgrid-ruby-4.2.1.gem.
  1. Create rails 5 sample application
cd /path/to/workspace
gem install rails -v 5.0.0
1 gem installed
rails new rails5_example
cd rails5_example
  1. Add local sendgrid-ruby package to the Gemfile
gem 'sendgrid-ruby', :path => '/Users/awwa/github/sendgrid-ruby'
  1. Install the local sendgrid-ruby package
bundle install
bundle update
:
Using sendgrid-ruby 4.2.1 from source at `/Users/awwa/github/sendgrid-ruby`
:
  1. Run Rails
rails s

  1. Run Sinatra
cd sendgrid-ruby
rackup
:
[2017-04-20 00:44:16] INFO  WEBrick::HTTPServer#start: pid=78410 port=9292

@gkats
Copy link

gkats commented Apr 24, 2017

Hello, just experienced the same problem that @skalee mentioned. Is there a plan for a release soon? How can we help?

Technical details:

  • sendgrid-ruby v4.3.0
  • ruby v2.3.1
  • rails v5.0.0
  • actionpack v5.0.0.1

@thinkingserious
Copy link
Contributor

Hi @gkats,

The best way to help is with a PR.

For now, I've added your vote to the issue to help it rise faster through our backlog.

Thanks!

With Best Regards,

Elmer

@gkats
Copy link

gkats commented Apr 25, 2017

@thinkingserious On it 😄

skalee added a commit to skalee/sendgrid-ruby that referenced this issue May 2, 2017
Fixes breaking change introduced in bd72df0
(sendgrid#160), and also fixes the
issue reported in sendgrid#159.
@skalee
Copy link
Contributor Author

skalee commented May 2, 2017

Now the only allowed dependency is a Sinatra's release candidate, which isn't good at all. And the Rack 1.x applications compatibility is broken (that is, still popular Rails 4 won't work).

@thinkingserious
Copy link
Contributor

Thanks for the fix @skalee!

@mull
Copy link

mull commented Jul 25, 2017

Why is the Sinatra dependency even there at all? It seems like that should be in a separate gem?

@thinkingserious
Copy link
Contributor

@mull,

You are correct, in the new version of this library, we will likely break out the inbound parser into its own gem like we just did with Node.js. If you have further feedback about the this SDKs refactor, please let us know here.

Thanks for taking the time to give us some feedback!

With Best Regards,

Elmer

This was referenced Sep 19, 2017
allhailskippy added a commit to allhailskippy/sendgrid-ruby that referenced this issue Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: bug bug in the library
Projects
None yet
Development

No branches or pull requests

5 participants