Skip to content

Add condition to check if ruby version is < 2.7#6977

Merged
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
emreerhan:emreerhan
Feb 1, 2020
Merged

Add condition to check if ruby version is < 2.7#6977
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
emreerhan:emreerhan

Conversation

@emreerhan
Copy link
Copy Markdown
Contributor

@emreerhan emreerhan commented Jan 30, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Fixes #6952

@emreerhan emreerhan requested a review from sjackman January 30, 2020 23:07
Comment thread Library/Homebrew/utils/ruby.sh Outdated
if [[ -z "$HOMEBREW_RUBY_PATH" || -n "$HOMEBREW_FORCE_VENDOR_RUBY" || "$ruby_version_new_enough" != "true" ]]
if [[ -n "$HOMEBREW_RUBY_PATH" && -z "$HOMEBREW_FORCE_VENDOR_RUBY" ]]
then
ruby_version_old_enough="$("$HOMEBREW_RUBY_PATH" --enable-frozen-string-literal --disable=gems,did_you_mean,rubyopt -rrubygems -e "puts Gem::Version.new(RUBY_VERSION.to_s.dup) < Gem::Version.new('$maximum_ruby_version')")"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm inclined to merge lines 46 and 51 into a single test: acceptable_ruby_version=…

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Great work so far! Not tested these changes locally but they should help get in the right direction!

Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Comment thread Library/Homebrew/utils/ruby.sh Outdated
Co-Authored-By: Mike McQuaid <mike@mikemcquaid.com>
Copy link
Copy Markdown
Contributor Author

@emreerhan emreerhan left a comment

Choose a reason for hiding this comment

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

I committed all of the suggested changes.

@emreerhan
Copy link
Copy Markdown
Contributor Author

Quick question, the MacOS CircleCI test failed because:

GitHub Bad credentials:HOMEBREW_GITHUB_API_TOKEN may be invalid or expired

It seems like this isn't an issue on my end, is it?

@emreerhan
Copy link
Copy Markdown
Contributor Author

Scratch my last comment. Seems like all checks have passed. When I have time later this weekend I'm going to try cloning this branch on a Docker instance w/ Ruby 2.7 installed to see if it works!

@MikeMcQuaid MikeMcQuaid merged commit d83afb1 into Homebrew:master Feb 1, 2020
@MikeMcQuaid
Copy link
Copy Markdown
Member

Thanks so much for your contribution! Without people like you submitting PRs we couldn't run this project. You rock, @emreerhan!

@sjackman
Copy link
Copy Markdown
Contributor

sjackman commented Feb 1, 2020

Thank you for your first contribution to Homebrew/brew, Emre! 🥇

@lock lock bot added the outdated PR was locked due to age label Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install on Manjaro Linux requires HOMEBREW_FORCE_VENDOR_RUBY

3 participants