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.

Sign up for GitHub

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

Jump to bottom

created rake restart task #18965

Merged
merged 1 commit into from
Feb 28, 2015
Merged

created rake restart task #18965

merged 1 commit into from
Feb 28, 2015

Conversation

hjoo
Copy link
Contributor

@hjoo 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).

@lxsameer lxsameer mentioned this pull request Feb 17, 2015
Closed
@eileencodes
Copy link
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.

@hjoo hjoo force-pushed the rake_restart branch 2 times, most recently from 205882c to 41f5c80 Compare February 26, 2015 17:10
eileencodes
eileencodes reviewed Feb 26, 2015
View reviewed changes
railties/test/application/rake/restart_test.rb
assert_not_equal prev_mtime, curr_mtime
end
end

Copy link
Member

Choose a reason for hiding this comment

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

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

@hjoo
Created rake restart task.
b181297 
Fixes rails#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
Merge pull request #18965 from hjoo/rake_restart
58aecb0 
created rake restart task
@eileencodes eileencodes merged commit 58aecb0 into rails:master Feb 28, 2015
@hundredwatt
Copy link
Contributor

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

@rafaelfranca
Copy link
Member

Yes, it can be dropped

@eileencodes
Copy link
Member

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

@matthewd
Copy link
Member

matthewd commented Mar 2, 2015

task :restart do

@eileencodes
Copy link
Member

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

@hjoo
Copy link
Contributor Author

hjoo commented Mar 2, 2015

Done! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
openacademy railties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hjoo @eileencodes @hundredwatt @rafaelfranca @matthewd @kaspth