Einav Cohen has posted comments on this change. Change subject: core, engine, webadmin: Shutdown/power off operations on a VM would ask for an optional reason ......................................................................
Patch Set 11: Gilad, I tend to agree with you, the only problem is that the 'Comment' field is already there and may already be used by users, and not necessarily for filling a shutdown reason. What if we are shutting down the VM, and the 'Comment' field is already filled with a value? - Override? probably not a good idea - the user will lose data. - Append to existing comment with a "~;~" separator (or similar)? we can, but what if the user will edit that comment? we won't necessarily be able to remove the shutdown reason once the VM is started (since the "~;~" separator is not there anymore since the user removed it, for example). We can parse the value to the 'Comment' part and the 'Reason' part (and display the 'Reason' as a read-only label, for example), but instead of parsing and concatenating, etc., it actually seems simpler to me to add a new field. Therefore, I think that we should proceed with the way that Ravi implemented it. Let me know what you think. thanks. -- To view, visit http://gerrit.ovirt.org/25633 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17645e5bc97a1a4d460956ec45f88524465dfd7b Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches