Skip to content

Support older rubies#41

Closed
johnsyweb wants to merge 3 commits intomasterfrom
paj/support-older-rubies
Closed

Support older rubies#41
johnsyweb wants to merge 3 commits intomasterfrom
paj/support-older-rubies

Conversation

@johnsyweb
Copy link
Copy Markdown
Contributor

@johnsyweb johnsyweb commented Nov 2, 2018

Context

https://github.com/envato/solid_octane_service/pull/190 and https://github.com/envato/solid_octane_service/pull/191 failed to annotate because solid_octane_service is on Ruby 2.3.5, which doesn't include ruby/rubygems#1659 . https://github.com/envato/solid_octane_service/pull/192 worked with this patch, which isn't perfect but I think is a reasonable fallback for old rubies.

Change

  • Use Gem::Versions#segments where Gem::Versions#canonical_segments isn't available.
  • Give @orien some long overdue credit for his amazing efforts
  • Fix Rubocop complaints (cherry-picked from Address gem build warnings... #40 )

Copy link
Copy Markdown
Member

@orien orien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @johnsyweb 😄

It's interesting that the build didn't pick that up. It runs against Ruby 2.3.

Ah, I see the Ruby 2.3 Docker image we use has the latest Rubygems installed.
https://github.com/docker-library/ruby/blob/9e6a656108d5227e5788dcf5f0e4c63e726dc3e6/2.3/stretch/Dockerfile#L13

@johnsyweb
Copy link
Copy Markdown
Contributor Author

Ah yes. Perhaps we should just fail loudly on old ruby gems.

@johnsyweb johnsyweb mentioned this pull request Nov 6, 2018
@johnsyweb
Copy link
Copy Markdown
Contributor Author

I think #46 is a better approach.

@johnsyweb johnsyweb closed this Nov 6, 2018
@johnsyweb johnsyweb deleted the paj/support-older-rubies branch November 6, 2018 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants