search

phaxio / phaxio-ruby

Ruby Gem for Phaxio API
http://bit.ly/phaxio-gem
MIT License
21 stars 34 forks source link

Update gemspec to support faraday >= 1.0.1 #29

Closed sjamog closed 3 years ago

sjamog commented 3 years ago

Faraday was updated to 1.0.0 last year- allow newer versions to be supported.

lucaong commented 3 years ago

This is quite important to allow Phaxio to be used with other popular gems that require Faraday 1.0.0 (such as Sentry). However, the version bounds look wrong to me: this would not allow, for example, 0.11 or 1.0.0, and instead allow a future 2.0.0 that might include breaking changes.

I sent a similar PR with version bounds that are, in my opinion, better.

jnankin commented 3 years ago

Thank you Luca!

lucaong commented 3 years ago

@jnankin just to be sure, it looks like you merged this PR and not this one. To me, this PR looks wrong, as it introduces a version range that is on one hand too restrictive (does not allow 0.11 or 1.0.0, which should all work according to semantic versioning), and on the other too permissive (allows 2.0.0 or 3.0.0, which according to semantic versioning can introduce breaking changes). The PR I sent fixes that, introducing the correct version bounds.

Of course, it's your decision, and if you determine that the other PR should not be merged you do not need to provide a reason for that 🙂 I am writing this just so, in case merging this PR was a mistake, you can fix it before releasing the gem.

jnankin commented 3 years ago

Appreciate the heads up. Must not have had my coffee yet. Thanks!