Idan Shaby has posted comments on this change. Change subject: uicommonweb: Attach virtual IDE disks new behavior ......................................................................
Patch Set 1: (9 comments) https://gerrit.ovirt.org/#/c/38663/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java: Line 1195: Line 1196: @DefaultStringValue("Activate") Line 1197: String activateVmDiskPopup(); Line 1198: Line 1199: @DefaultStringValue("When the VM is up, only non IDE disks are activated after attached.") > maybe: Done Line 1200: String activateVmDiskPopupToolTip(); Line 1201: Line 1202: @DefaultStringValue("Alias") Line 1203: String aliasVmDiskTable(); https://gerrit.ovirt.org/#/c/38663/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AttachDiskModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AttachDiskModel.java: Line 153: Line 154: Frontend.getInstance().runMultipleActions(actionTypes, paramerterList, callbacks, null, this); Line 155: } Line 156: Line 157: public EntityModel<String> getDynamicWarning() > move '{' to EOL Done Line 158: { Line 159: return dynamicWarning; Line 160: } Line 161: Line 158: { Line 159: return dynamicWarning; Line 160: } Line 161: Line 162: public void setDynamicWarning(EntityModel<String> value) > same Done Line 163: { Line 164: dynamicWarning = value; Line 165: } Line 166: Line 188: } Line 189: return selectedDisks; Line 190: } Line 191: Line 192: private void initDynamicWarning() { > no real need for this method, i.e. just put the content in ctr.. Done Line 193: setDynamicWarning(new EntityModel<String>()); Line 194: getDynamicWarning().setIsAvailable(false); Line 195: } Line 196: Line 193: setDynamicWarning(new EntityModel<String>()); Line 194: getDynamicWarning().setIsAvailable(false); Line 195: } Line 196: Line 197: private boolean isIDEDiskSelected(List<EntityModel<DiskModel>> selectedDisks) { > looks weird :) s/isIDEDiskSelected/isSelectedDiskInterfaceIDE Done Line 198: for (EntityModel<DiskModel> selectedDisk : selectedDisks) { Line 199: if (selectedDisk.getEntity().getDisk().getDiskInterface() == DiskInterface.IDE) { Line 200: return true; Line 201: } Line 215: getDynamicWarning().setEntity(CONSTANTS.ideDisksWillBeAttachedButNotActivated()); Line 216: getDynamicWarning().setIsAvailable(true); Line 217: } Line 218: else { Line 219: getDynamicWarning().setIsAvailable(false); > you can just move it to line 211 instead Done Line 220: } Line 221: } Line 222: else { Line 223: getDynamicWarning().setIsAvailable(false); Line 219: getDynamicWarning().setIsAvailable(false); Line 220: } Line 221: } Line 222: else { Line 223: getDynamicWarning().setIsAvailable(false); > same Done Line 224: } Line 225: } Line 226: Line 227: private void addViewsSelectedItemsChangedListener() { Line 223: getDynamicWarning().setIsAvailable(false); Line 224: } Line 225: } Line 226: Line 227: private void addViewsSelectedItemsChangedListener() { > simply 'addSelectedItemsChangedListener' should be clear enough Done Line 228: IEventListener<EventArgs> selectionChangedListener = new IEventListener<EventArgs>() { Line 229: @Override Line 230: public void eventRaised(Event<? extends EventArgs> ev, Object sender, EventArgs args) { Line 231: updateDynamicWarning(); https://gerrit.ovirt.org/#/c/38663/1/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java: Line 2412: Line 2413: @DefaultStringValue("When the VM is running, cannot activate a disk attached with IDE interface.") Line 2414: String cannotHotPlugDiskWithIdeInterface(); Line 2415: Line 2416: @DefaultStringValue("Selected IDE disk/s will be attached but will need to be activated manually after the VM is shut down.") > "The selected IDE disk(s) will be attached, but will not be activated. In o Done Line 2417: String ideDisksWillBeAttachedButNotActivated(); Line 2418: Line 2419: @DefaultStringValue("Cannot activate disk, VM should be in Down, Paused or Up status.") Line 2420: String cannotPlugDiskIncorrectVmStatus(); -- To view, visit https://gerrit.ovirt.org/38663 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I187c74c7c39f438dfa5e81942f4a6aec41323830 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
