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

Updates gemspec to use pessimistic versioning on dependencies #815

Merged
merged 6 commits into from
Sep 5, 2013

Conversation

davefp
Copy link
Contributor

@davefp davefp commented Aug 21, 2013

Let's talk about versioning.

The majority of ruby gems use (and the rest should use) the Rational Versioning Policy defined by RubyGems.org.

Currently, active_merchant has several sub-optimal version definitions on its dependencies. Things like

s.add_dependency('active_utils', '>= 1.0.2')

and

s.add_dependency('nokogiri', "< 1.6.0")

Since patch and minor version changes should not break backwards compatibility, let's ditch these and instead start using pessimistic versioning in all applicable cases. The above examples become

s.add_dependency('active_utils', '~> 1.0')
s.add_dependency('nokogiri', "~> 1.6")

Much cleaner.

I've updated the gemspec to use pessimistic versioning for all of the runtime dependencies. I've left the development dependencies alone for now as I'm not sure how best to proceed there.

@jduff
Copy link
Contributor

jduff commented Aug 22, 2013

ci is erroring out, any ideas what's up?

@davefp
Copy link
Contributor Author

davefp commented Aug 22, 2013

Yeah, I messed up the commit. Working on it now.

David Underwood added 2 commits August 22, 2013 10:24
* master:
  gemspec: Dont force nokogiri lesser than 1.6.0
  Eway: Update supported countries
  Fixed a bug in the handling of multiple charges to the same customer

Conflicts:
	activemerchant.gemspec
@csaunders
Copy link

Version locking to active_support 3.2 is going to make it impossible for any developers using rails 4.0 to use this gem.

We shouldn't be using anything in active_support hard requires 3.2 so I say remove that.

@davefp
Copy link
Contributor Author

davefp commented Aug 22, 2013

Looks like active_support is constrained by the various Gemfile_railsxx files. Dumb question: Are these used in the released gem somehow, or are they just there for Travis to test against different Rails versions?

@davefp
Copy link
Contributor Author

davefp commented Aug 22, 2013

Never mind, I answered my own question.

David Underwood added 4 commits August 22, 2013 13:10
* master:
  Allow nokogiri as low as 1.4.7
  Balanced: Adjust BalancedGateway::Error parent
  SecureNet: Allow some optional fields
  WebPay: add tests to modify currency unit in JPY from yen to sen.
  WebPay: modify refund method to support the new currency unit in JPY.
  WebPay: add unit tests to check the currency unit.
  WebPay: update a test API key.
  WebPay: modify the currency unit in JPY from yen (1) to sen (1/100).
  Update Stripe gateway to reflect current currency support
  added missing authorization_code param to response
  Moneris: Add verification_value support
  Credit deprecation test for litle.
  Litle: Deprecate credit method in favor of refund.
  Pass the primary response into the run method instead of tacking it on the end.
  Stripe: Make the actual refund response primary rather than the fee refund.
  Allow changing the primary response of a MultiResponse to the first one run.
  Firstdata E4: Include missing address information for AVS and CVV results.

Conflicts:
	activemerchant.gemspec
@davefp
Copy link
Contributor Author

davefp commented Aug 29, 2013

@jduff @csaunders Everything is passing now.

I had to make some concessions to support all the rails versions we target in Travis, but all the runtime requirements now have an intelligent lower and upper version boundary so there shouldn't be any surprises when the next major version of a gem we depend on comes out.

@csaunders
Copy link

Fabulous!

shipit

davefp added a commit that referenced this pull request Sep 5, 2013
Updates gemspec to use pessimistic versioning on dependencies
@davefp davefp merged commit f09138e into activemerchant:master Sep 5, 2013
@davefp davefp deleted the pessimistic_versioning branch September 5, 2013 15:10
@jhawthorn
Copy link
Contributor

👎 💣 version constraints should not be added willy nilly.

Having nokogiri locked down in active_merchant has already been an enormous pain for me and others (#813, #765). As active_merchant is widely used, versions should not be so strict.

jhawthorn added a commit to StemboltHQ/active_merchant that referenced this pull request Sep 5, 2013
@davefp
Copy link
Contributor Author

davefp commented Sep 6, 2013

@jhawthorn This patch doesn't force old versions of Nokogiri (or the other libs, as far as I'm aware). It's designed to protect users against major version changes (e.g. 2.x.x to 3.x.x) in dependencies that will break AM by proxy.

In the Nokogiri example specifically, the gemspec says this: s.add_dependency('nokogiri', "~> 1.4"). This means AM will accept anything >= 1.4.0 and < 2.0.0. The current version is 1.6.0, and so falls into the valid range and will be installed by bundler. If version 2.0.0 comes out and works fine then sure, we can relax the constraints.

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

Successfully merging this pull request may close these issues.

4 participants