Tal Nisan has posted comments on this change.
Change subject: frontend: add the import glance image support
......................................................................
Patch Set 5: (5 inline comments)
Minor comments, please fix and you have my +2 ;)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/ImportExportRepoImageModel.java
Line 27: import java.util.Collections;
Line 28: import java.util.List;
Line 29:
Line 30:
Line 31: public abstract class ImportExportRepoImageModel extends EntityModel
implements ICommandTarget {
Perhaps we can call it ImportExportRepoImageBaseModel or
ImportExportRepoImageBase? This is not a standalone model and the name may be
misleading
Line 32:
Line 33: protected static EventDefinition
selectedItemChangedEventDefinition;
Line 34:
Line 35: static {
Line 146:
Line 147: protected void updateControlsAvailability() {
Line 148: getDataCenter().setIsChangable(!getDataCenter().getIsEmpty());
Line 149:
getStorageDomain().setIsChangable(!getStorageDomain().getIsEmpty());
Line 150: getQuota().setIsChangable(!getQuota().getIsEmpty());
Do we need all this overhead of setting and getting the isEmpty attribute? If
the list is empty the select box will not be changeable anyway since we have no
options to select which is practically the same
Line 151:
getOkCommand().setIsExecutionAllowed(!getStorageDomain().getIsEmpty());
Line 152: setMessage(getStorageDomain().getIsEmpty() ?
constants.noStorageDomainAvailableMsg() : null);
Line 153: }
Line 154:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageIsoListModel.java
Line 183: @Override
Line 184: public void executeCommand(UICommand command) {
Line 185: super.executeCommand(command);
Line 186:
Line 187: if (command == getImportImagesCommand()) {
Basically it is in that specific can cause what is transferred as a parameter
is the same command object, but that's not a good practice, better to change to
equals indeed but change the order to make it null safe case the command
parameter is null
Line 188: importImages();
Line 189: }
Line 190: else if (command.getName().equals("Cancel")) { //$NON-NLS-1$
Line 191: cancel();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/ImportExportImagePopupView.java
Line 106: public String getText(Object image) {
Line 107: if (image instanceof RepoImage) {
Line 108: return ((RepoImage) image).getRepoImageTitle();
Line 109: }
Line 110: return "(Unknown)"; //$NON-NLS-1$
Yes, CommonApplicationConstants.unknown() already exists for that
Line 111: }
Line 112: }, constants.fileNameIso());
Line 113: imageList.addEntityModelColumn(new
EntityModelTextColumn<Object>() {
Line 114: @Override
Line 115: public String getText(Object image) {
Line 116: if (image instanceof RepoImage) {
Line 117: return ((RepoImage)
image).getFileType().toString();
Line 118: }
Line 119: return "(Unknown)"; //$NON-NLS-1$
Same
Line 120: }
Line 121: }, constants.typeIso());
Line 122: imageList.addEntityModelColumn(new
DiskSizeColumn<EntityModel>() {
Line 123: @Override
--
To view, visit http://gerrit.ovirt.org/16083
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I084dadfd208255738140f093ffc8ec136f83d7f6
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
