Mike Kolesnik has posted comments on this change.
Change subject: engine: adding "migration" property to NetworkCluster
......................................................................
Patch Set 9: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/AttachNetworkToVdsGroupCommand.java
Line 72: && changesAreClusterCompatible()
Line 73: && logicalNetworkExists();
Line 74:
Line 75: if (canDo) {
Line 76: NetworkClusterValidator validator =
I would just put this block in a separate method (i.e. validateAttachment())
and do something like:
return vdsGroupExists()
&& changesAreClusterCompatible()
&& logicalNetworkExists()
&& validateAttachment();
Line 77: new NetworkClusterValidator(getNetworkCluster(),
getVdsGroup().getcompatibility_version());
Line 78: canDo =
Line 79: (!NetworkUtils.isManagementNetwork(getNetwork())
|| validate(validator.managementNetworkAttachment(getNetworkName())))
Line 80: &&
validate(validator.migrationPropertySupported(getNetworkName()));
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/UpdateNetworkOnClusterCommand.java
Line 69: }
Line 70:
Line 71: @Override
Line 72: protected boolean canDoAction() {
Line 73: boolean canDo;
Same comment as in the add attachmeent command
Line 74: canDo = validate(networkClusterAttachmentExists());
Line 75:
Line 76: if (canDo) {
Line 77: Version clusterVersion =
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/NetworkClusterValidatorTest.java
Line 77: false);
Line 78: }
Line 79:
Line 80: @Test
Line 81: public void
migrationNetworkNotManagementWhenMigrationNetworkSupported() throws Exception {
You should also test the cases when migration network is not set and not
supported, and not set but is supported, which both should also be valid.
Line 82: migrationNetworkSupportTest(isValid(),
Line 83: true,
Line 84: true,
Line 85: false);
Line 86: }
Line 87:
Line 88: private void migrationNetworkSupportTest(Matcher<ValidationResult>
matcher,
Line 89: boolean migrationNetworkSupported,
Line 90: boolean isMigration,
The naming convention that we use dictates that boolean
variable/field/parameter names don't start with "is", only getters.
Therefore, no need to name the parameters "isSomething", just "something"
Line 91: boolean isManagement) {
Line 92:
mockConfigRule.mockConfigValue(ConfigValues.MigrationNetworkEnabled, version,
migrationNetworkSupported);
Line 93: mockConfigRule.mockConfigValue(ConfigValues.ManagementNetwork,
isManagement ? NETWORK_NAME
Line 94: : NETWORK_NAME + NOT_MANAGEMENT);
--
To view, visit http://gerrit.ovirt.org/12962
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199b1f48b6d6096db2425c594e88455640dc626c
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
