created rake restart task #18965

Merged
merged 1 commit into from Feb 28, 2015

Conversation

Projects
None yet
6 participants
@hjoo
Contributor

hjoo commented Feb 17, 2015

Created rake restart task to address issue #18876. Was uncertain if spring restart required if tmp/restart.txt will be on Spring watch list (referencing #18874).

@eileencodes

This comment has been minimized.

Show comment
Hide comment
@eileencodes

eileencodes Feb 17, 2015

Member

@hjoo I think you can add some tests for this rake task in railties/test/application/rake/ or railties/test/commands/ - honestly not sure which is best. Take a look at what kind of commands are there and base your decision on that.

Member

eileencodes commented Feb 17, 2015

@hjoo I think you can add some tests for this rake task in railties/test/application/rake/ or railties/test/commands/ - honestly not sure which is best. Take a look at what kind of commands are there and base your decision on that.

+ assert_not_equal prev_mtime, curr_mtime
+ end
+ end
+

This comment has been minimized.

@eileencodes

eileencodes Feb 26, 2015

Member

@hjoo can you remove this extra line. Otherwise I think this is looking good 😄

@eileencodes

eileencodes Feb 26, 2015

Member

@hjoo can you remove this extra line. Otherwise I think this is looking good 😄

railties/CHANGELOG.md
@@ -1,3 +1,9 @@
+* Created rake restart task.

This comment has been minimized.

@kaspth

kaspth Feb 26, 2015

Member

It would be nice with an extra sentence that says something like: "Restarts your Rails app by touching the "tmp/restart.txt" file 😄

@kaspth

kaspth Feb 26, 2015

Member

It would be nice with an extra sentence that says something like: "Restarts your Rails app by touching the "tmp/restart.txt" file 😄

This comment has been minimized.

@eileencodes

eileencodes Feb 26, 2015

Member

👍 good call, I agree @kaspth

@eileencodes

eileencodes Feb 26, 2015

Member

👍 good call, I agree @kaspth

Created rake restart task.
Fixes #18876. Rake restart touches `tmp/restart.txt` to restart
application on next request. Updated tests and documentation
accordingly.

eileencodes added a commit that referenced this pull request Feb 28, 2015

@eileencodes eileencodes merged commit 58aecb0 into rails:master Feb 28, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hundredwatt

This comment has been minimized.

Show comment
Hide comment
@hundredwatt

hundredwatt Mar 2, 2015

Does this task need to depend on the :environment task? Seems like that could be dropped (ie task :restart do) for faster execution.

Does this task need to depend on the :environment task? Seems like that could be dropped (ie task :restart do) for faster execution.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Mar 2, 2015

Member

Yes, it can be dropped

Member

rafaelfranca commented Mar 2, 2015

Yes, it can be dropped

@eileencodes

This comment has been minimized.

Show comment
Hide comment
@eileencodes

eileencodes Mar 2, 2015

Member

@hjoo can you send a new PR that just does task :restart do instead of task restart: :environment do

Member

eileencodes commented Mar 2, 2015

@hjoo can you send a new PR that just does task :restart do instead of task restart: :environment do

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd Mar 2, 2015

Member

task :restart do

Member

matthewd commented Mar 2, 2015

task :restart do

@eileencodes

This comment has been minimized.

Show comment
Hide comment
@eileencodes

eileencodes Mar 2, 2015

Member

Thanks @matthewd it's early here 😁 need moar ☕️

Member

eileencodes commented Mar 2, 2015

Thanks @matthewd it's early here 😁 need moar ☕️

@hjoo

This comment has been minimized.

Show comment
Hide comment
@hjoo

hjoo Mar 2, 2015

Contributor

Done! :)

Contributor

hjoo commented Mar 2, 2015

Done! :)

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