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

Remove `ruby RUBY_VERSION` from generated Gemfile #5803

Merged
merged 3 commits into from May 13, 2017

Conversation

Projects
None yet
5 participants
@ashmaroli
Member

ashmaroli commented Jan 21, 2017

I've had ruby RUBY_VERSION in all my Gemfiles generated by jekyll new on Windows. Ignored it because it never seemed to cause any issue.
Proposing interpolating the variable based on this comment to trigger a Travis build and check if this is a fix..

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jan 21, 2017

Member

okay.. dont know what burnt Travis. But local tests on my Windows system with Ruby 2.3.3 and Bundler 1.14.0 did not raise issues with ruby RUBY_VERSION

This PR may lead to future issues by having the subsequent blogs generated locked to a certain Ruby version.
Leaving it open for further feedback

label: Discussion, dont-merge-yet

Member

ashmaroli commented Jan 21, 2017

okay.. dont know what burnt Travis. But local tests on my Windows system with Ruby 2.3.3 and Bundler 1.14.0 did not raise issues with ruby RUBY_VERSION

This PR may lead to future issues by having the subsequent blogs generated locked to a certain Ruby version.
Leaving it open for further feedback

label: Discussion, dont-merge-yet

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 31, 2017

Member

We should probably just get rid of this line. I think it's only useful because Heroku requires it, but even then it's not that useful. I use the ruby pragma in Gemfile exactly never.

Member

parkr commented Mar 31, 2017

We should probably just get rid of this line. I think it's only useful because Heroku requires it, but even then it's not that useful. I use the ruby pragma in Gemfile exactly never.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Mar 31, 2017

Member

We should probably just get rid of this line

I like that idea. Would it be better if it were commented out instead?

Member

ashmaroli commented Mar 31, 2017

We should probably just get rid of this line

I like that idea. Would it be better if it were commented out instead?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 31, 2017

Member

Would it be better if it were commented out instead?

Why comment it out? That seems like useless bloat.

Member

parkr commented Mar 31, 2017

Would it be better if it were commented out instead?

Why comment it out? That seems like useless bloat.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Mar 31, 2017

Member

That seems like useless bloat.

LOL! okay, will remove it entirely.

Member

ashmaroli commented Mar 31, 2017

That seems like useless bloat.

LOL! okay, will remove it entirely.

remove :ruby from generated Gemfile
The `ruby #{RUBY_VERSION}` is only required when deploying to Heroku
@DirtyF

DirtyF approved these changes Mar 31, 2017

@pathawks

🎉

@ashmaroli ashmaroli changed the title from Interpolate RUBY_VERSION in generated Gemfile to Remove `ruby RUBY_VERSION` from generated Gemfile Apr 1, 2017

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Apr 1, 2017

Member

The do-not-merge label ought to be removed, no?

Member

ashmaroli commented Apr 1, 2017

The do-not-merge label ought to be removed, no?

@pathawks pathawks removed the do-not-merge label Apr 1, 2017

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Apr 1, 2017

Member

@ashmaroli Yeah! Who added that anyway?
…oh 😶

Member

pathawks commented Apr 1, 2017

@ashmaroli Yeah! Who added that anyway?
…oh 😶

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF May 13, 2017

Member

@jekyllbot: merge +dev

Member

DirtyF commented May 13, 2017

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 1d55b70 into jekyll:master May 13, 2017

2 checks passed

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

jekyllbot added a commit that referenced this pull request May 13, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 13, 2017

Member

@jekyllbot: merge +dev

This isn't a dev change, this is a change to the way jekyll new works.

Member

parkr commented May 13, 2017

@jekyllbot: merge +dev

This isn't a dev change, this is a change to the way jekyll new works.

@ashmaroli ashmaroli deleted the ashmaroli:Gemfile-patch branch Oct 29, 2017

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