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

Lock nokogiri to 1.7.x for Ruby 2.1 #6140

Merged
merged 4 commits into from Jun 18, 2017

Conversation

Projects
None yet
5 participants

@parkr parkr added the tests label Jun 15, 2017

parkr added some commits Jun 15, 2017

@parkr parkr self-assigned this Jun 15, 2017

@parkr parkr requested a review from pathawks Jun 15, 2017

@pathawks

LGTM Pending passing tests 👍

Show outdated Hide outdated Gemfile
@@ -22,7 +22,8 @@ group :test do
gem "cucumber", "~> 2.1"
gem "jekyll_test_plugin"
gem "jekyll_test_plugin_malicious"
gem "nokogiri"
# nokogiri v1.8 does not work with ruby 2.1 and below
gem "nokogiri", RUBY_VERSION > "2.1" ? "~> 1.7" : "~> 1.7.0"

This comment has been minimized.

@DirtyF

DirtyF Jun 16, 2017

Member

Shouldn't it be something like:
gem "nokogiri", RUBY_VERSION < "2.1" ? "1.7" : "1.8"

@DirtyF

DirtyF Jun 16, 2017

Member

Shouldn't it be something like:
gem "nokogiri", RUBY_VERSION < "2.1" ? "1.7" : "1.8"

Show outdated Hide outdated Gemfile
@@ -22,7 +22,8 @@ group :test do
gem "cucumber", "~> 2.1"
gem "jekyll_test_plugin"
gem "jekyll_test_plugin_malicious"
gem "nokogiri"
# nokogiri v1.8 does not work with ruby 2.1 and below
gem "nokogiri", RUBY_VERSION > "2.1" ? "~> 1.7" : "~> 1.7.0"

This comment has been minimized.

@ashmaroli

ashmaroli Jun 16, 2017

Member

no @DirtyF , it should instead be:

gem "nokogiri", RUBY_VERSION >= "2.2" ? "~> 1.7" : "~> 1.7.0"

> 2.1 will include 2.1.1, 2.1.2 etc

@ashmaroli

ashmaroli Jun 16, 2017

Member

no @DirtyF , it should instead be:

gem "nokogiri", RUBY_VERSION >= "2.2" ? "~> 1.7" : "~> 1.7.0"

> 2.1 will include 2.1.1, 2.1.2 etc

This comment has been minimized.

@pathawks

pathawks Jun 16, 2017

Member

> "2.1" is equivalent to >= "2.2"

?

@pathawks

pathawks Jun 16, 2017

Member

> "2.1" is equivalent to >= "2.2"

?

This comment has been minimized.

@ashmaroli

ashmaroli Jun 16, 2017

Member

no it isn't

@ashmaroli

ashmaroli Jun 16, 2017

Member

no it isn't

This comment has been minimized.

@ashmaroli

ashmaroli Jun 16, 2017

Member

> 2.1 will include 2.1.1, 2.1.2... 2.2.x, 2.3.x, 2.4.x
>= 2.2 will include only 2.2.0, 2.2.1,..., 2.4.x

@ashmaroli

ashmaroli Jun 16, 2017

Member

> 2.1 will include 2.1.1, 2.1.2... 2.2.x, 2.3.x, 2.4.x
>= 2.2 will include only 2.2.0, 2.2.1,..., 2.4.x

This comment has been minimized.

@pathawks

pathawks Jun 16, 2017

Member

Is it different practically or only semantically? 😕

@pathawks

pathawks Jun 16, 2017

Member

Is it different practically or only semantically? 😕

This comment has been minimized.

@ashmaroli

ashmaroli Jun 16, 2017

Member

both.. Appveyor still fails with the last commit because, its still installing 1.8.0 instead..

@ashmaroli

ashmaroli Jun 16, 2017

Member

both.. Appveyor still fails with the last commit because, its still installing 1.8.0 instead..

This comment has been minimized.

@pathawks

pathawks Jun 16, 2017

Member

TIL 🍻

@pathawks

pathawks Jun 16, 2017

Member

TIL 🍻

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 18, 2017

Member

💚

@jekyllbot: merge +dev

Member

parkr commented Jun 18, 2017

💚

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 01dd356 into master Jun 18, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the fix-nokogiri-for-ruby-2-1 branch Jun 18, 2017

jekyllbot added a commit that referenced this pull request Jun 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment