Daniel Erez has posted comments on this change.

Change subject: webadmin: preview snapshot with memory popup
......................................................................


Patch Set 6: (5 inline comments)

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmSnapshotPreviewPopupWidget.java
Line 49:     }
Line 50: 
Line 51:     void localize(CommonApplicationConstants constants) {
Line 52:         
memoryEditor.setLabel(constants.virtualMachineSnapshotPreviewPopupMemoryLabel());
Line 53:         
messageLabel.setText(ConstantsManager.getInstance().getMessages().snapshotContainsMemory());
so any reason not to move it into CommonApplicationMessages for consistency?
Line 54:     }
Line 55: 
Line 56:     @Override
Line 57:     public void edit(final SnapshotModel model) {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmSnapshotListModel.java
Line 426: 
Line 427:             model.getCommands().add(new UICommand("OnPreview", this) 
//$NON-NLS-1$
Line 428:                
.setTitle(ConstantsManager.getInstance().getConstants().ok())
Line 429:                .setIsDefault(true));
Line 430:             UICommand cancelCommand = new UICommand("Cancel", this) 
//$NON-NLS-1$
ok, so line 433 is probably redundant
Line 431:                
.setTitle(ConstantsManager.getInstance().getConstants().cancel())
Line 432:                .setIsCancel(true);
Line 433:             model.getCommands().add(cancelCommand);
Line 434:             model.setCancelCommand(cancelCommand);


Line 441:     private void OnPreview() {
Line 442:         Snapshot snapshot = (Snapshot) getSelectedItem();
Line 443: 
Line 444:         if (snapshot == null)
Line 445:         {
left curly brace should be at the end of line...
Line 446:             cancel();
Line 447:             return;
Line 448:         }
Line 449: 


Line 468:     private void newEntity()
Line 469:     {
Line 470:         VM vm = (VM) getEntity();
Line 471:         if (vm == null || getWindow() != null)
Line 472:         {
same
Line 473:             return;
Line 474:         }
Line 475: 
Line 476:         SnapshotModel model = new SnapshotModel();


Line 807:     }
Line 808: 
Line 809:     private boolean memorySnapshotSupported;
Line 810: 
Line 811:     protected void updateIsMemorySnapshotSupported()
so why not calling this method from preview?
i.e. no need to update it on each 'setEntity' and no need for the private 
'memorySnapshotSupported' flag (just return the boolean)
Line 812:     {
Line 813:         if (getEntity() == null)
Line 814:         {
Line 815:             return;


-- 
To view, visit http://gerrit.ovirt.org/15686
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I18c99b22b33256e6bcf1cb9b1151e908e27ded8d
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to