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

Re-add support for Ruby 1.9.3 #14

Merged
merged 2 commits into from
Aug 19, 2015
Merged

Re-add support for Ruby 1.9.3 #14

merged 2 commits into from
Aug 19, 2015

Conversation

parkr
Copy link
Member

@parkr parkr commented Aug 18, 2015

Fixes #13, follow-up to #11.

We need a fix for #7 (comment) that works in Ruby 1.9.3.

/cc @gr4y

@gr4y
Copy link
Contributor

gr4y commented Aug 18, 2015

I will take a look at that when my Mac finished compiling ruby 1.9.3, which might take some more minutes. But I just asked my self if the issue #7 will be un-fixed by fixing #13.

Net::OpenTimeout is not available in Ruby 1.9.3 which indicates that the timeout which fixed #7 won't be available either in net/http in 1.9.3.

@gr4y
Copy link
Contributor

gr4y commented Aug 18, 2015

net/http in Ruby 1.9.3 throws an TimeoutError when it times out. In Ruby 2.0 net/http throws an special Exception for each timeout.

So by just removing the Net::ReadTimeout and Net::OpenTimeout from the code I ran into undefined method 'empty?' for #<URI::HTTPS:0x007fd203cd4240>

So I tried wrapping the begin..rescue..end block into another begin..rescue..end block and catching NameError, which worked out, but results in missing noscript and failing specs even when the network connection is working fine.

@gr4y
Copy link
Contributor

gr4y commented Aug 19, 2015

So what's your plan on this? I seriously don't know what to do except removing the feature introduced in #7 or reverting the commits made in #11 to fix that.

@parkr
Copy link
Member Author

parkr commented Aug 19, 2015

So I tried wrapping the begin..rescue..end block into another begin..rescue..end block and catching NameError

Lol a NameError is usually not something you want to catch... If empty? doesn't exist somewhere, then some code is borked and we need to figure out where it went wrong there.

I think I fixed it by just spiking those error classes out.

parkr added a commit that referenced this pull request Aug 19, 2015
@parkr parkr merged commit 6f12c59 into master Aug 19, 2015
@parkr parkr deleted the fix-for-1-9-3 branch August 19, 2015 21:01
parkr added a commit that referenced this pull request Aug 19, 2015
@gr4y
Copy link
Contributor

gr4y commented Aug 20, 2015

@parkr I guessed that catching NameError won't be a smart idea. By the way: I haven't heard the term "spiking out" ever before. Seems I am a little out of the loop when it comes to ruby. Sorry about that. 😭

@parkr
Copy link
Member Author

parkr commented Aug 20, 2015

By the way: I haven't heard the term "spiking out" ever before. Seems I am a little out of the loop when it comes to ruby. Sorry about that.

It's absolutely no problem! "Spike out" is an idiom. When I say "Spike out code", I mean "write the initial version". A working scaffold. Sorry for that! I should be more considerate of people who don't speak idiomatic English.

@gr4y
Copy link
Contributor

gr4y commented Aug 21, 2015

It's a rather cool way to fix backwards compatibility problems like that one, so I learned something new here. 😄

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v1.3.1 requires Ruby >= 2.0, breaks Jekyll install on at least ubuntu 14.04
3 participants