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

now follows 'safe' http->https redirs for images #223

Merged
merged 1 commit into from Nov 3, 2015

Conversation

Projects
None yet
4 participants
@Bill
Contributor

Bill commented Oct 30, 2015

The wordpress importer was failing to follow http->https redirections for my images (so it was failing to download them). This was due to open-uri not following these redirs.

Added the open_uri_redirections gem and changed the open(uri) call to:

open(uri, allow_redirections: :safe)

Also changed error logging method to say "Error" instead of "Errorr".

@Bill

This comment has been minimized.

Show comment
Hide comment
@Bill

Bill Oct 30, 2015

Contributor

Looks like the Travis CI failure is on Ruby 1.9.3:

Gem::InstallError: jekyll requires Ruby version >= 2.0.0.

I did not add the dependency on jekyll, so this must've been failing before my PR.

Contributor

Bill commented Oct 30, 2015

Looks like the Travis CI failure is on Ruby 1.9.3:

Gem::InstallError: jekyll requires Ruby version >= 2.0.0.

I did not add the dependency on jekyll, so this must've been failing before my PR.

@Bill

This comment has been minimized.

Show comment
Hide comment
@Bill

Bill Oct 30, 2015

Contributor

Ah Jekyll 3.0.0 (the latest) requires Ruby 2:

https://rubygems.org/gems/jekyll/versions/3.0.0

Jekyll 3.0.0 was released on October 27, 2014 (three days ago). The previous version, 2.5.3, released in December of 2014 did not require Ruby 2.

Is it time to turn off the Ruby 1.9.3 tests in Travis CI?

Contributor

Bill commented Oct 30, 2015

Ah Jekyll 3.0.0 (the latest) requires Ruby 2:

https://rubygems.org/gems/jekyll/versions/3.0.0

Jekyll 3.0.0 was released on October 27, 2014 (three days ago). The previous version, 2.5.3, released in December of 2014 did not require Ruby 2.

Is it time to turn off the Ruby 1.9.3 tests in Travis CI?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Nov 2, 2015

Member

Not sure I love the added dependency. Can we do this manually?

Member

parkr commented Nov 2, 2015

Not sure I love the added dependency. Can we do this manually?

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Nov 2, 2015

Member

IMHO, this isn't worth reimplementing ourselves. Why would we do this ourselves versus just adding the dependency?

Member

mattr- commented Nov 2, 2015

IMHO, this isn't worth reimplementing ourselves. Why would we do this ourselves versus just adding the dependency?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Nov 3, 2015

Member

IMHO, this isn't worth reimplementing ourselves. Why would we do this ourselves versus just adding the dependency?

Well having a new runtime dependency and asking for the dependency just for this one converter is different, I guess. My thought is to not have a runtime dependency. I guess we can ask for it for this converter, but redirects are so easy to do with Net::HTTP? We do a lot of that in jekyll-gist.

Member

parkr commented Nov 3, 2015

IMHO, this isn't worth reimplementing ourselves. Why would we do this ourselves versus just adding the dependency?

Well having a new runtime dependency and asking for the dependency just for this one converter is different, I guess. My thought is to not have a runtime dependency. I guess we can ask for it for this converter, but redirects are so easy to do with Net::HTTP? We do a lot of that in jekyll-gist.

@parkr parkr merged commit 63f15c2 into jekyll:master Nov 3, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

parkr added a commit that referenced this pull request Nov 3, 2015

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Nov 3, 2015

Member

We do use a lot of Net::HTTP in jekyll-gist, but I didn't see any redirect handling code there.

Member

mattr- commented Nov 3, 2015

We do use a lot of Net::HTTP in jekyll-gist, but I didn't see any redirect handling code there.

parkr added a commit that referenced this pull request Dec 22, 2015

parkr added a commit that referenced this pull request Jan 4, 2016

@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.