Tal Nisan has posted comments on this change.
Change subject: engine: Unify Guid and NGuid
......................................................................
Patch Set 1: Looks good to me, but someone else must approve
(8 inline comments)
Minor comments, aside for that there are cases in test classes that you are
using Guid.createGuidFromStringDefaultEmpty on static strings, I guess that
this because you used a sed for all occurrences of Guid.createGuidFromString to
be change to Guid.createGuidFromStringDefaultEmpty, in the interest of good
order those should be used with Guid.createGuidFromString since they cannot be
null, but not a big issue if you don't want to since the behavior is the same
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/ReplaceGlusterVolumeBrickCommandTest.java
Line 43: private Guid clusterId = new
Guid("c0dd8ca3-95dd-44ad-a88a-440a6e3d8106");
Line 44: private Guid serverId = new
Guid("d7f10a21-bbf2-4ffd-aab6-4da0b3b2ccec");
Line 45: private Guid volumeId1 = new
Guid("8bc6f108-c0ef-43ab-ba20-ec41107220f5");
Line 46: private Guid volumeId2 = new
Guid("b2cb2f73-fab3-4a42-93f0-d5e4c069a43e");
Line 47: private Guid volumeId3 =
Guid.createGuidFromStringDefaultEmpty("000000000000-0000-0000-0000-00000003");
Why not use new Guid(...) like the 4 other above? Or just move them to use
createGuidFromString?
Line 48: private Guid volumeId4 =
Guid.createGuidFromStringDefaultEmpty("000000000000-0000-0000-0000-00000004");
Line 49:
Line 50: /**
Line 51: * The command under test.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
Line 17: private Guid id = new Guid();
Line 18:
Line 19: @ValidName(message = "VALIDATION.DATA_CENTER.NAME.INVALID", groups
= { CreateEntity.class, UpdateEntity.class })
Line 20: @Size(min = 1, max =
BusinessEntitiesDefinitions.DATACENTER_NAME_SIZE)
Line 21: private String name = ""; // GREGM prevents NPE
Very redundant comment..
Line 22:
Line 23: @Size(max = BusinessEntitiesDefinitions.GENERAL_MAX_SIZE)
Line 24: @Pattern(regexp = ValidationUtils.ONLY_ASCII_OR_NONE,
Line 25: message = "VALIDATION.DATA_CENTER.DESCRIPTION.INVALID",
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
Line 1284:
Line 1285: private Version privateGuestAgentVersion;
Line 1286:
Line 1287: /**
Line 1288: * assumption: Qumranet Agent version stored in app_list by
"Qumranet Agent" name. Qumranet Agent version,
I'd remove this in another patch, "Qumranet" is way deprecated ;)
Line 1289: * received from vds in format : a.b.d there is no major
revision received from vds - always 0
Line 1290: * @see {@link Version}
Line 1291: */
Line 1292: public Version getGuestAgentVersion() {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostModel.java
Line 1227: param.setUser(isPrimary ? (String)
getPmUserName().getEntity() : (String) getPmSecondaryUserName().getEntity());
Line 1228: param.setPassword(isPrimary ? (String)
getPmPassword().getEntity() : (String) getPmSecondaryPassword().getEntity());
Line 1229: param.setStoragePoolId(cluster.getStoragePoolId().getValue()
!= null ? cluster.getStoragePoolId()
Line 1230: .getValue()
Line 1231: .getValue() : Guid.Empty);
Are the spaces here on purpose?
Line 1232: param.setFencingOptions(new
ValueObjectMap(getPmOptionsMap(), false));
Line 1233: param.setPmProxyPreferences(getPmProxyPreferences());
Line 1234:
Line 1235: Frontend.RunQuery(VdcQueryType.GetNewVdsFenceStatus, param,
new AsyncQuery(this, new INewAsyncCallback() {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/MoveOrCopyDiskModel.java
Line 82:
Line 83: protected abstract String getNoActiveTargetDomainMessage();
Line 84:
Line 85: protected abstract MoveOrCopyImageGroupParameters createParameters(
Line 86: Guid sourceStorageDomaiGuid,
I guess a result from the sed? Please change it back to sourceStorageDomainGuid
Line 87: Guid destStorageDomaiGuid,
Line 88: DiskImage disk);
Line 89:
Line 90: public MoveOrCopyDiskModel() {
Line 83: protected abstract String getNoActiveTargetDomainMessage();
Line 84:
Line 85: protected abstract MoveOrCopyImageGroupParameters createParameters(
Line 86: Guid sourceStorageDomaiGuid,
Line 87: Guid destStorageDomaiGuid,
And here
Line 88: DiskImage disk);
Line 89:
Line 90: public MoveOrCopyDiskModel() {
Line 91: setAllDisks(new ArrayList<DiskModel>());
Line 295: }
Line 296:
Line 297: Guid destStorageDomaiGuid = destStorageDomain.getId();
Line 298: addMoveOrCopyParameters(parameters,
Line 299: sourceStorageDomaiGuid,
same here
Line 300: destStorageDomaiGuid,
Line 301: disk);
Line 302: }
Line 303:
Line 304: return parameters;
Line 305: }
Line 306:
Line 307: protected void
addMoveOrCopyParameters(ArrayList<VdcActionParametersBase> parameters,
Line 308: Guid sourceStorageDomaiGuid,
Same here
Line 309: Guid destStorageDomaiGuid,
Line 310: DiskImage disk) {
Line 311:
Line 312: MoveOrCopyImageGroupParameters params =
createParameters(sourceStorageDomaiGuid, destStorageDomaiGuid, disk);
--
To view, visit http://gerrit.ovirt.org/15691
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4af6d353ef27394ff902f374de19682628c52739
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
