Martin Mucha has posted comments on this change. Change subject: core: renamed method ......................................................................
Patch Set 40: (1 comment) https://gerrit.ovirt.org/#/c/36571/40/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java: Line 158: protected ManagementNetworkUtil getManagementNetworkUtil() { Line 159: return Injector.get(ManagementNetworkUtil.class); Line 160: } Line 161: Line 162: public ValidationResult notRemovingManagementNetwork() { > As I mentioned in an earlier patch, you're checking whether the network is however if we look at generated error message — "…~cannot remove~…", this context cannot be changed. I.e. former method name allowed this method to be mistakenly used in broader context. You actually cannot use this method to test if network is management and if is complaint about it removal. That's why I added 'Removing' into method name. Ok, so lets agree, that both names are wrong. Proposals: • networkBeingRemovedIsNotManagementNetwork • removedNetworkIsNotManagementNetwork Line 163: return isManagementNetwork() Line 164: ? new ValidationResult(VdcBllMessages.NETWORK_CANNOT_REMOVE_MANAGEMENT_NETWORK, Line 165: getNetworkNameReplacement()) Line 166: : ValidationResult.VALID; -- To view, visit https://gerrit.ovirt.org/36571 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7469ab4578d689b89a32daaf2e53c9d2de1de70a Gerrit-PatchSet: 40 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
