Raise error if ruby 1.9.3 is detected. #1838

Merged
merged 1 commit into from Jul 21, 2014

Projects

None yet

3 participants

@seuros
Collaborator
seuros commented Jul 10, 2014

@mperham, Let me know if i need to reformulate the error message.

@mperham mperham and 2 others commented on an outdated diff Jul 10, 2014
lib/sidekiq.rb
@@ -1,4 +1,5 @@
# encoding: utf-8
+fail 'Unsupported Ruby Version' if RUBY_PLATFORM != 'java' && RUBY_VERSION < '2.0.0'
@mperham
mperham Jul 10, 2014 owner

I'm not sure this should go here (where it will cause Rails server to fail to start also) or in cli.rb (where it will just kill the sidekiq server). wdyt?

What about Rubinius or other future VMs? I think it might be better to explicitly look for MRI, rather than exclude java.

@mperham
mperham Jul 10, 2014 owner

And the error should be clearer, e.g. "Sidekiq #{Sidekiq::VERSION} does not support Ruby 1.9."

@seuros
seuros Jul 10, 2014 collaborator

I did it here to have an early error. For me both places are ok, i would like opinions of others.
ping @brandonhilkert , @jonhyman , @ryanlecompte , @jc00ke ...

Rubinius and others don't work because of the unicode heart anyway. And even when they do (rbx-edge), their ruby version is over 2.0.0+

@aprescott
aprescott Jul 10, 2014

If the intent is to prevent MRI < 2.0, then it should check for MRI instead of operating on this assumption, I think.

@seuros
Collaborator
seuros commented Jul 21, 2014

@mperham, It ok if i merge this pr ? It will remove 1.9.3 from travis.yml

@mperham
Owner
mperham commented Jul 21, 2014

You should check explicitly for MRI, not Java.

On Jul 21, 2014, at 2:12, Abdelkader Boudih notifications@github.com wrote:

@mperham, It ok if i merge this pr ? It will remove 1.9.3 from travis.yml


Reply to this email directly or view it on GitHub.

@seuros
Collaborator
seuros commented Jul 21, 2014

But that will not raise the error in other flavors of ruby before 2.0.0.
The code is to allow only jruby to have 1.9.3.
WDYT ?

@mperham mperham merged commit 27fc885 into mperham:master Jul 21, 2014

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
@mperham
Owner
mperham commented Jul 21, 2014

Fair enough.

@seuros seuros deleted the seuros:fail_on_ruby1_9 branch Jul 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment