Ayal Baron has posted comments on this change.
Change subject: webadmin: Import VM dialog redesign (#845947)
......................................................................
Patch Set 5: (9 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
Line 55:
Line 56: @DefaultStringValue("Storage")
Line 57: String singleDestinationStorage();
Line 58:
Line 59: @DefaultStringValue("Default Storage")
What I was asking in my comment for patcheset 2 is why isn't this string
'Default Storage Domain'
Line 60: String defaultStorage();
Line 61:
Line 62: @DefaultStringValue(" and Quota")
Line 63: String singleQuota();
Line 1003:
Line 1004: @DefaultStringValue("Direct LUN")
Line 1005: String lunDisksLabel();
Line 1006:
Line 1007: @DefaultStringValue("Currnet")
s/Currnet/Current/
shouldn't the string be 'Current Quota' though?
Line 1008: String currentQuota();
Line 1009:
Line 1010: @DefaultStringValue("Name")
Line 1011: String elementName();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java
Line 246: ImportCloneModel cloneModel = (ImportCloneModel)
getConfirmWindow();
Line 247: if ((Boolean) cloneModel.getApplyToAll().getEntity()) {
Line 248: if (!(Boolean) cloneModel.getNoClone().getEntity()) {
Line 249: String suffix = (String)
cloneModel.getSuffix().getEntity();
Line 250: if (!validateSuffix(suffix, cloneModel.getSuffix())) {
need a TODO Iinside validateSuffix probably) that says we need support for
running numbers
Line 251: return;
Line 252: }
Line 253: for (VM vm : vmsInSetupMap.values()) {
Line 254: vm.setvm_name(vm.getvm_name() + suffix);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportData.java
Line 42: this.selectedQuota = selectedQuota;
Line 43: }
Line 44:
Line 45: public VolumeType getSelectedVolumeType() {
Line 46: if (!(Boolean) collapseSnapshots.getEntity() ||
selectedVolumeType == null) {
I figured out what's bugging me about this line. The logic here is in not
semantics.
it should be:
if ((Boolean) collapseSnapshots.getEntity() && selectedVolumeType != null) {
return selectedVolumeType;
}
return getVolumeType();
This way it is clearer that the only case where user can affect the volume type
is if collapse is selected and user changed the value.
Line 47: return getVolumeType();
Line 48: }
Line 49: return selectedVolumeType;
Line 50: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java
Line 279: && disk.getvolume_format() ==
VolumeFormat.RAW
Line 280: && getDiskImportData(disk.getId()) != null
Line 281: &&
(getDiskImportData(disk.getId()).getSelectedStorageDomain()
Line 282:
.getstorage_type().isBlockDomain())) {
Line 283: getProblematicItems().add(vm);
getProblematicItems doesn't mean anything.
Method name should be getDisksToConvert or something
Line 284: break;
Line 285: }
Line 286: }
Line 287: }
Line 506: public boolean isObjectInSetup(Object vm) {
Line 507: if (alreadyInSystemVmMap == null) {
Line 508: return false;
Line 509: }
Line 510: return alreadyInSystemVmMap.containsKey(((VM) vm).getId());
still need to get rid of this
Line 511: }
Line 512:
Line 513: public boolean isQuotaEnabled() {
Line 514: return getStoragePool() != null
....................................................
File
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java
Line 1496:
Line 1497: @DefaultStringValue("Assign Disk Quota")
Line 1498: String assignQuotaForDisk();
Line 1499:
Line 1500: @DefaultStringValue("Some Templates' disks which are missing.
Suggested solutions: "
1. This is grammatically incorrect "Some Templates' disks which are missing."
2. In '2' the operation is Import not Export.
3. Need to list which templates are missing.
Should be something like:
Some imported VMs depend on one or more templates which are not available in
the system.
Either:
1. Import VMs with 'collapse snapshots' option
2. Import missing templates first and then try importing the VMs again
Missing templates: ...
Line 1501: + "1. Preserving Template-Based structure - "
Line 1502: + "Make sure the relevant Templates exist on the
relevant Data-Center and Import the VMs again."
Line 1503: + "2. Using non-Template-Based structure (less optimal
storage-wise) - "
Line 1504: + "Export the VMs again using the Collapse Snapshots
option.")
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java
Line 2160:
Line 2161: @DefaultStringValue("This operation might be unrecoverable and
destructive!")
Line 2162: String storageForceCreatePopupWarningLabel();
Line 2163:
Line 2164: @DefaultStringValue("Quota Cluster")
This should be 'Cluster quota' and not 'Quota cluster' (which is grammatically
incorrect)
Line 2165: String quotaCluster();
Line 2166:
Line 2167: @DefaultStringValue("Quota Storage")
Line 2168: String quotaStorage();
Line 2163:
Line 2164: @DefaultStringValue("Quota Cluster")
Line 2165: String quotaCluster();
Line 2166:
Line 2167: @DefaultStringValue("Quota Storage")
same
Line 2168: String quotaStorage();
Line 2169:
Line 2170: @DefaultStringValue("Assign Quota")
Line 2171: String assignQuota();
--
To view, visit http://gerrit.ovirt.org/7313
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b6551b235c66e6a32db0f8842cd8e410d6e0318
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Haim Ateya <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[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
