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

Test against Ruby 2.4.0 #5687

Merged
merged 6 commits into from Mar 31, 2017

Conversation

Projects
None yet
6 participants
@ashmaroli
Member

ashmaroli commented Dec 25, 2016

Test against the latest version of Ruby: v2.4.0

ashmaroli added some commits Dec 25, 2016

use compatible versions of gems
  - json: "~> 2.0"
    # flori/json#303 (comment)

  - pygments.rb: "~> 1.1"
Show outdated Hide outdated .travis.yml
- &ruby1 2.4.0
- &ruby2 2.3.1
- &ruby3 2.2.5
- &ruby4 2.1.9

This comment has been minimized.

@yous

yous Jan 2, 2017

Contributor

How about bumping previous versions? Current versions are 2.3.3, 2.2.6, and 2.1.10. https://www.ruby-lang.org/en/downloads/

@yous

yous Jan 2, 2017

Contributor

How about bumping previous versions? Current versions are 2.3.3, 2.2.6, and 2.1.10. https://www.ruby-lang.org/en/downloads/

This comment has been minimized.

@ashmaroli

ashmaroli Jan 2, 2017

Member

that's for a separate PR. This PR is only about Ruby 2.4.
You're welcome to create a PR to bump the rvm versions we currently test against.
😃

@ashmaroli

ashmaroli Jan 2, 2017

Member

that's for a separate PR. This PR is only about Ruby 2.4.
You're welcome to create a PR to bump the rvm versions we currently test against.
😃

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 11, 2017

Member

It looks like the pygments.rb update is causing issues. Does pygments.rb 1.0 work on Ruby 2.1, 2.2, and 2.3, too? If it does, then maybe we should just update!

Member

parkr commented Feb 11, 2017

It looks like the pygments.rb update is causing issues. Does pygments.rb 1.0 work on Ruby 2.1, 2.2, and 2.3, too? If it does, then maybe we should just update!

@parkr parkr added the blocked label Feb 11, 2017

Show outdated Hide outdated Gemfile
gem "json", "~> 2.0"
gem "pygments.rb", "~> 1.1"
else
gem "pygments.rb", "~> 0.6.0" unless RUBY_ENGINE == "jruby"

This comment has been minimized.

@parkr

parkr Feb 11, 2017

Member

Can we use pygments.rb v1 with Ruby 2.1,2,3,4 instead of using 2 versions?

@parkr

parkr Feb 11, 2017

Member

Can we use pygments.rb v1 with Ruby 2.1,2,3,4 instead of using 2 versions?

This comment has been minimized.

@ashmaroli

ashmaroli Feb 11, 2017

Member

we can.. but I wasn't sure if the change would affect Jekyll internally in any way..
I pushed a new commit to use just one version of pygments.rb

From the logs, it looks like we just need to change our assertions to match the new pygments syntax. But I don't want to undertake that task.

So, you're free to close this PR or assign someone to directly modify this branch.

@ashmaroli

ashmaroli Feb 11, 2017

Member

we can.. but I wasn't sure if the change would affect Jekyll internally in any way..
I pushed a new commit to use just one version of pygments.rb

From the logs, it looks like we just need to change our assertions to match the new pygments syntax. But I don't want to undertake that task.

So, you're free to close this PR or assign someone to directly modify this branch.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 13, 2017

Member

Thank you! We should definitely upgrade the pygments.rb dependency as you already did, but it has caused a number of test failures. It would be preferable to see the upgrade & test failures fix in a separate PR so we can keep this focused on the 2.4 upgrade.

It also occurs to me that we should check 2.4 on AppVeyor as well! If that's a possibility, it would be nice to keep Travis & AppVeyor in harmony.

Member

parkr commented Feb 13, 2017

Thank you! We should definitely upgrade the pygments.rb dependency as you already did, but it has caused a number of test failures. It would be preferable to see the upgrade & test failures fix in a separate PR so we can keep this focused on the 2.4 upgrade.

It also occurs to me that we should check 2.4 on AppVeyor as well! If that's a possibility, it would be nice to keep Travis & AppVeyor in harmony.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Feb 13, 2017

Member

I think we can't test with Ruby 2.4.0 on AppVeyor yet as the RubyInstaller project has not yet released a corresponding binary for Windows.

/cc @XhmikosR

Member

ashmaroli commented Feb 13, 2017

I think we can't test with Ruby 2.4.0 on AppVeyor yet as the RubyInstaller project has not yet released a corresponding binary for Windows.

/cc @XhmikosR

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Feb 13, 2017

Contributor

Yeah, not possible. We need 2.4.0, but see oneclick/rubyinstaller#348

Contributor

XhmikosR commented Feb 13, 2017

Yeah, not possible. We need 2.4.0, but see oneclick/rubyinstaller#348

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 13, 2017

Member

@ashmaroli @XhmikosR That settles it then! 👍 We'll wait on rubyinstaller for AppVeyor support and in the meantime we'll hope that the 2.4.0 tests on unix-like platforms will work for any brave souls who install Ruby 2.4.0 on their Windows machines.

Member

parkr commented Feb 13, 2017

@ashmaroli @XhmikosR That settles it then! 👍 We'll wait on rubyinstaller for AppVeyor support and in the meantime we'll hope that the 2.4.0 tests on unix-like platforms will work for any brave souls who install Ruby 2.4.0 on their Windows machines.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Feb 13, 2017

Member

From the look of the test failures, it looks like the only thing that going to break is the pygments HTML output.. doesn't seem to be a serious issue, IMO

Member

ashmaroli commented Feb 13, 2017

From the look of the test failures, it looks like the only thing that going to break is the pygments HTML output.. doesn't seem to be a serious issue, IMO

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 13, 2017

Member

From the look of the test failures, it looks like the only thing that going to break is the pygments HTML output.. doesn't seem to be a serious issue, IMO

Yeah, agreed. I would like to contain that change in one separate PR so we can always trace back. Feel free to simply fix the tests so the expectations match! The HTML that pygments outputs seems good enough to me.

Member

parkr commented Feb 13, 2017

From the look of the test failures, it looks like the only thing that going to break is the pygments HTML output.. doesn't seem to be a serious issue, IMO

Yeah, agreed. I would like to contain that change in one separate PR so we can always trace back. Feel free to simply fix the tests so the expectations match! The HTML that pygments outputs seems good enough to me.

@parkr parkr referenced this pull request Mar 7, 2017

Merged

Upgrade pygments to v1.x #5937

@parkr parkr merged commit 6123175 into jekyll:master Mar 31, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 31, 2017

Member

Thank you!

Member

parkr commented Mar 31, 2017

Thank you!

parkr added a commit that referenced this pull request Mar 31, 2017

@ashmaroli ashmaroli deleted the ashmaroli:ruby-2.4.0 branch Mar 31, 2017

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