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

Added bin/update script to update application automatically #20972

Merged
merged 1 commit into from Aug 7, 2015

Conversation

@meinac
Copy link
Contributor

@meinac meinac commented Jul 21, 2015

I think this a helpful script for team members after pulling the changes from remote repository. They can add whatever they want to update their application in this script.

@rafaelfranca rafaelfranca added this to the 5.0.0 milestone Jul 27, 2015
@dhh
Copy link
Member

@dhh dhh commented Jul 27, 2015

👍

# Add necessary update steps to this file.

puts '== Installing dependencies =='
system 'bundle install'

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Jul 28, 2015
Member

It might be interesting to take in consideration the system! implementation added by other PR, to abort in case of a failure.

@carlosantoniodasilva
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva commented Jul 28, 2015

Sounds good to me.

@@ -1,3 +1,7 @@
* Add `bin/update` script to update application automatically

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Jul 28, 2015
Member

I think we need to be a bit more descriptive here. What does "update application automatically" really mean? Is that different from deploy? Just to make intents clear.

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Aug 6, 2015

I've declared this method here and I don't like what I did. I think this is a bad example to show developers to repeat yourself. Maybe we can define this method in active_support to use in other places, I didn't think declaring this method inside of a file in bin folder because someone can remove it. Any advice?

It is a script so there is no problem of repeating it. Over DRy is also a problem

@rafaelfranca
rafaelfranca reviewed Aug 6, 2015
View changes
railties/lib/rails/generators/rails/app/templates/bin/update Outdated
# Add necessary update steps to this file.

puts '== Installing dependencies =='
system! 'bundle install'

This comment has been minimized.

@rafaelfranca

rafaelfranca Aug 6, 2015
Member

We need to call bundle check first.

@rafaelfranca
rafaelfranca reviewed Aug 6, 2015
View changes
railties/lib/rails/generators/rails/app/templates/bin/update Outdated
system! 'bundle install'

puts "\n== Updating database =="
system! 'ruby bin/rake db:migrate'

This comment has been minimized.

@rafaelfranca

rafaelfranca Aug 6, 2015
Member

there is no need to use ruby here. bin/rake will work.

@meinac
Copy link
Contributor Author

@meinac meinac commented Aug 6, 2015

@rafaelfranca I fixed the problems you've pointed, WDYT about this now?


puts '== Installing dependencies =='
system! 'gem install bundler --conservative'
system! 'bundle check' or system! 'bundle install'

This comment has been minimized.

@matthewd

matthewd Aug 7, 2015
Member

Per #20992, system! never returns false

use system!

fix changelog

use bundle check first and use rake

use system instead system! for bundle check
@meinac
Copy link
Contributor Author

@meinac meinac commented Aug 7, 2015

@matthewd Thanks for the point, I've fixed it.

rafaelfranca added a commit that referenced this pull request Aug 7, 2015
Added bin/update script to update application automatically
@rafaelfranca rafaelfranca merged commit dcc4783 into rails:master Aug 7, 2015
@rafaelfranca rafaelfranca modified the milestones: 5.0.0 [temp], 5.0.0 Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.