-
Notifications
You must be signed in to change notification settings - Fork 983
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
fixes #5969 - deleting of EC2 guests - confusing message #1490
Conversation
@domcleal, is this destroy message true for all types of compute resources, or just EC2? |
@isratrade I don't know, please test. |
For the sake of avoiding per-VM provider deletion messages, may I suggest using different phrasing, something like: "It may take a few minutes to take effect". |
@witlessbird, how is the message 'The virtual machine will be deleted in a few minutes' connected with per-VM provider. Also note that the message is on the |
@@ -70,7 +70,7 @@ def pause | |||
def destroy | |||
@compute_resource = find_compute_resource(:destroy_compute_resources_vms) | |||
if @compute_resource.destroy_vm params[:id] | |||
process_success({ :success_redirect => compute_resource_vms_path(@compute_resource) }) | |||
process_success({ :success_redirect => compute_resource_vms_path(@compute_resource), :success_msg => _('The virtual machine will be deleted in a few minutes') }) |
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.
if the deletion only takes seconds (or less) for libvirt, then this message "in a few minutes" may be confusing.
Should we change it to
"The virtual machine is being deleted in the background"
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.
Yeah, or simply "The virtual machine is being deleted."
re-committed with message "The virtual machine is being deleted." |
Tested locally, works as described. +1 to merge |
Merged as 9d9523c for Foreman 1.6.0, thanks @isratrade. |
No description provided.