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

Support Gem::Version properly #1024

Closed
wants to merge 1 commit into from

Conversation

knewter
Copy link

@knewter knewter commented Mar 24, 2013

I haven't tested this with rails < 4-master, but this fixes breakage where Rails.version now returns a Gem::Version and these comparisons fail there. This is the offending commit afaict: rails/rails@c07e151

@jnicklas
Copy link
Collaborator

That seems weird. Comparing string sizes like this seems incorrect to me. For example:

>> "3.1.2.beta1" > "3.1.2"
=> true

@knewter
Copy link
Author

knewter commented Mar 25, 2013

That's an excellent point, though I have no idea what to_f did with that. I didn't even consider that case, though it's obvious now. How is Gem::Version intended to be compared?

@abotalov
Copy link
Collaborator

Here's even more weird example:

'3.1.2' > '3.1.10' # => true 

What about <=>:
Rails.version >= Gem::Version(3.0)

@jnicklas
Copy link
Collaborator

@abotalov if Rails previously returned something other than a Gem::Version that might break on older versions.

@abotalov
Copy link
Collaborator

@jnicklas What about:

if Rails.version.is_a?(String)
  Gem::Version.new(Rails.version) >= Gem::Version.new(3.0)
else
 Rails.version >= Gem::Version.new(3.0)
end

@jnicklas
Copy link
Collaborator

I'm not even sure what it was before, was it really a string?

@abotalov
Copy link
Collaborator

@jnicklas

1.9.3p286 :015 > Rails.version
 => "3.2.11" 

@eval
Copy link

eval commented Mar 26, 2013

How about:

Gem::Version.new(Rails.version) >= Gem::Version.new(3)

This is compatible with Rails.version being a string or Gem::Version

@ghost ghost closed this in 78e3a48 Mar 26, 2013
jnicklas pushed a commit that referenced this pull request Mar 26, 2013
Due to rails/rails#8501, we can no longer case the Rails version to a float. This version should hopefully work on both Rails 4 master and on Rails 3.
@jnicklas
Copy link
Collaborator

I cherry picked this onto 2.0_stable and released it as 2.0.3 together with another tweak. A simple bundle update capybara should now fix this issue.

This pull request was closed.
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