-
Notifications
You must be signed in to change notification settings - Fork 896
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
Memory checkbox will not show when VM is not up #12678
Conversation
@masayag please review |
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.
👍 LGTM
👍 from the prvoiders side @martinpovolny can you check the UI part and merge if ok? |
@borod108 please resolve the confilict |
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
When creating a snapshot there is not reason to show the option to save runtime memory if the VM is down. https://bugzilla.redhat.com/show_bug.cgi?id=1375850
af78467
to
de813a1
Compare
@durandom conflict resolved |
Checked commit borod108@de813a1 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
@durandom again had a strange CI error https://travis-ci.org/ManageIQ/manageiq/jobs/177665079#L693 |
@martinpovolny I'm 👍 please merge if you are ok too |
@martinpovolny hi, can we merge this? |
@himdel I'm good from the providers side, if you are from the UI, can you merge this? |
@borod108 please update the description to include a screenshot and instructions where to find this in the UI. (Also seems like you left a part of the PR template in the description..) Not merging before I can test it :) (though it does look good) |
@himdel added images is that ok? |
@borod108 thanks, I guess I can try finding it by the images. But if you could also add real instructions (go there, click that, ..), would be even better :) |
@himdel sure you are right. Will this be ok:
|
@borod108 thanks, that really helps! :) So .. the only question that remains is this..
|
it's actually only true for rhev?!
Could it be, you grepped for something else? |
@durandom, @borod108 OK, thanks, I'll check further. I was grepping for the But seems like we shouldn't both be defininng the method, overriding it, and creating it via Either way, I don't have any RHEV VM that'd have the button (snapshots in the summary) clickable.. |
Oh, @himdel you are absolutely right. This slipped my review on #11418 @himdel can you merge this as is or do you want to test it out with a VM? |
@durandom Sure, merged, as long as it can happen.. :) |
Memory checkbox will not show when VM is not up (cherry picked from commit d05b5f2) https://bugzilla.redhat.com/show_bug.cgi?id=1403981
Euwe backport details:
|
When a VM is powered off there should not be a "save memory" checkbox:
When a VM is powered off there should be a "save memory" checkbox:
When creating a snapshot there is not reason to show
the option to save run time memory if the VM is down.
https://bugzilla.redhat.com/show_bug.cgi?id=1375850