-
Notifications
You must be signed in to change notification settings - Fork 13
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
Install foreman as part of bin/setup #169
Conversation
Reason for Change ================= * It is required for local development but NOT in the Gemfile. * This could be confusing. * Why not just set it up in `bin/setup`? Changes ======= * Add a step to install the `foreman` gem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🌮
system("bundle check") || system!("bundle install") | ||
end | ||
|
||
step "Installing foreman for controlling local servers" do | ||
system!("gem install foreman") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we check if foreman is already installed before doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the down-side to placing it in the Gemfile explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to lean in the direction of "Things that live completely outside the application should not be in the Gemfile" (Say, rerun
or mailcatcher
) but have no strong preferences here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we check if foreman is already installed before doing this?
I thought of this, but if we call gem install
and it is installed, bundler will just upgrade it. So I'm inclined to leave it, unless you feel like that is an unpleasant surprise.
Touché!
…On Tue, Sep 5, 2017 at 1:50 PM, Jessie A. Young ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In bin/setup
<#169 (comment)>
:
> system("bundle check") || system!("bundle install")
end
+ step "Installing foreman for controlling local servers" do
+ system!("gem install foreman")
thoughtbot/suspenders#330
<thoughtbot/suspenders#330> :)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#169 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABuIuskVdhDLRZCH9z4PvdsOIV28zMKks5sfYnxgaJpZM4PNSwx>
.
|
Reason for Change
bin/setup
?Changes
foreman
gem.