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

Allow system to have a higher version of PhantomJS installed. #25

Closed
wants to merge 1 commit into from

Conversation

Aevin1387
Copy link

I have 1.9.6 installed on my machine, but due to the version check to see if PhantomJS is installed, this installs 1.9.2 on my machine. Since this gem is forced on me from the jasmine-rails gem, I have created a patch.

@jsatk
Copy link

jsatk commented Jan 27, 2014

I'm in the same boat as @Aevin1387. This would be very beneficial to have the PhantomJS gem use 1.9.6.

@@ -32,7 +32,8 @@ def system_phantomjs_version
end

def system_phantomjs_installed?
system_phantomjs_version == Phantomjs.version
version = system_phantomjs_version
version ? version >= Phantomjs.version : false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version test fails for comparisons such as '1.10.0' > '1.9.6'.

I have partially finished branch which adds support for specifying the acceptable version range, and the default setting would have the effect you're going for here. I'll take a look at it again this weekend.

@rossta
Copy link

rossta commented Feb 17, 2014

👍

@jabr
Copy link
Collaborator

jabr commented Mar 5, 2014

I very much appreciate this PR, but I'm going to close it and refer everyone to PR #31.

That PR does this by default and makes it all configurable.

@jabr jabr closed this Mar 5, 2014
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