Skip to content
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

Use platforms instead of install_if #8140

Merged
merged 3 commits into from Apr 27, 2020
Merged

Use platforms instead of install_if #8140

merged 3 commits into from Apr 27, 2020

Conversation

dleidert
Copy link
Contributor

With :install_if bundler will check for the gems even if they are not required
on our platform. To prevent users from having this issue use :platforms
instead: https://lists.debian.org/debian-ruby/2020/04/msg00062.html

fixes #8139

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

LGTM

@DirtyF DirtyF requested a review from a team April 26, 2020 05:32
@DirtyF DirtyF added the fix label Apr 26, 2020
@DirtyF DirtyF added this to the 4.1 milestone Apr 26, 2020
@ashmaroli
Copy link
Member

@dleidert We also have a similar usage in our dev Gemfile. Could you patch those lines as well..?
Thanks.

jekyll/Gemfile

Lines 83 to 88 in 2bc1e04

# Windows and JRuby does not include zoneinfo files, so bundle the tzinfo-data gem
# and associated library
install_if -> { RUBY_PLATFORM =~ %r!mingw|mswin|java! } do
gem "tzinfo", "~> 1.2"
gem "tzinfo-data"
end

Gemfile Outdated Show resolved Hide resolved
@DirtyF DirtyF requested a review from a team April 27, 2020 08:06
@ashmaroli
Copy link
Member

@DirtyF Do you think this patch qualifies to be backported for v4.0.x..?

@DirtyF
Copy link
Member

DirtyF commented Apr 27, 2020

@ashmaroli Not if we release 4.1 sooner than later. As you see fit.

@DirtyF
Copy link
Member

DirtyF commented Apr 27, 2020

@jekyll: merge +fix

@jekyllbot jekyllbot merged commit acf9259 into jekyll:master Apr 27, 2020
@jekyllbot jekyllbot added the bug label Apr 27, 2020
jekyllbot added a commit that referenced this pull request Apr 27, 2020
@dleidert dleidert deleted the fix_8139_new_gemfile branch April 27, 2020 14:51
@jekyll jekyll locked and limited conversation to collaborators Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The created Gemfile (jekyll new) should use platforms instead if install_if
4 participants