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

Fixed problematic Sinatra dependency #161

Merged
merged 2 commits into from
May 2, 2017

Conversation

skalee
Copy link
Contributor

@skalee skalee commented May 2, 2017

Brings back Rails 4 compatibility (and Rack 1.x applications, in general), also removes release candidate version constraint (both broken in #160). Moreover, ensures that tests are run against two major Sinatra versions, which should protect from compatibility issues in future, somewhat.

Related issue: #159.

Fixes breaking change introduced in bd72df0
(sendgrid#160), and also fixes the
issue reported in sendgrid#159.
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label May 2, 2017
@SendGridDX
Copy link

SendGridDX commented May 2, 2017

CLA assistant check
All committers have signed the CLA.

@skalee
Copy link
Contributor Author

skalee commented May 2, 2017

Alternatively, I would consider extracting Sinatra-based parts to another gem, so that the basic Sendgrid-Ruby gem does not introduce many dependencies.

@thinkingserious
Copy link
Contributor

@skalee,

The Sinatra dependency is only needed for those setting up an Inbound Parse Webhook server.

What do you think about these options:

  1. Move the Inbound Parse helper to it's own repo
  2. Make the dependencies for that helper optional

I'm favoring option 2 as a quick fix for now.

Thoughts?

@thinkingserious thinkingserious merged commit b518327 into sendgrid:master May 2, 2017
@thinkingserious
Copy link
Contributor

Hello @skalee,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants