-
Notifications
You must be signed in to change notification settings - Fork 65
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
DEV: rename the "upgrade" route to "update" and change references. #210
Conversation
config/routes.rb
Outdated
post "/docker/update" => "admin#update" | ||
delete "/docker/update" => "admin#reset_update" |
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 don't see updates to the controller actions, do we need to do that as well?
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.
Good catch! I now reverted the changes in route methods. I'm not going rename the method names in this PR (only the routes).
lib/docker_manager/upgrader.rb
Outdated
@@ -257,9 +257,9 @@ def status(val) | |||
publish("status", val) | |||
end | |||
|
|||
def log_version_upgrade | |||
def log_version_update |
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.
We renamed the method, but I don’t see another instance of the method in the list of changes. Do we have more places to update?
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 didn't intent to update that method name too. I fixed it now. I doubled checked all other method names again.
This PR will rename the route "
/admin/upgrade
" to "/admin/update
" and it will add a redirect for the old route.Follow-up to the PR: #208