Mike Kolesnik has posted comments on this change. Change subject: webadmin: Add gateway & DNS servers fields ......................................................................
Patch Set 1: (5 comments) http://gerrit.ovirt.org/#/c/25359/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ExternalSubnetModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ExternalSubnetModel.java: Line 101: subnet.setGateway(getGateway().getEntity()); Line 102: Line 103: List<String> dnsServers = new ArrayList<String>(); Line 104: for (EntityModel<String> dnsServer : getDnsServers().getItems()) { Line 105: if (dnsServer.getEntity() != null && !dnsServer.getEntity().isEmpty()) { > You can use StringHelper.isNullOrEmpty() Done Line 106: dnsServers.add(dnsServer.getEntity()); Line 107: } Line 108: } Line 109: subnet.setDnsServers(dnsServers); Line 115: getCidr().validateEntity(new IValidation[] { ipv4 Line 116: ? new CidrValidation() Line 117: : new NotEmptyValidation() }); Line 118: getIpVersion().validateSelectedItem(new IValidation[] { new NotEmptyValidation() }); Line 119: if (getGateway().getEntity() != null && !getGateway().getEntity().isEmpty() && ipv4) { > You can use StringHelper.isNullOrEmpty() Done Line 120: getGateway().validateEntity(new IValidation[] { new IpAddressValidation() }); Line 121: } Line 122: Line 123: boolean dnsServersValid = true; Line 121: } Line 122: Line 123: boolean dnsServersValid = true; Line 124: for (EntityModel<String> dnsServer : getDnsServers().getItems()) { Line 125: if (dnsServer.getEntity() != null && !dnsServer.getEntity().isEmpty() && ipv4) { > You can use StringHelper.isNullOrEmpty() Done Line 126: dnsServer.validateEntity(new IValidation[] { new IpAddressValidation() }); Line 127: } Line 128: dnsServersValid &= dnsServer.getIsValid(); Line 129: } Line 124: for (EntityModel<String> dnsServer : getDnsServers().getItems()) { Line 125: if (dnsServer.getEntity() != null && !dnsServer.getEntity().isEmpty() && ipv4) { Line 126: dnsServer.validateEntity(new IValidation[] { new IpAddressValidation() }); Line 127: } Line 128: dnsServersValid &= dnsServer.getIsValid(); > Why do you use bitwise and? It makes sense to calculate the final result this way, and since it's a boolean field it's equivalent to a longer assignment (which won't contribute to the readability in this specific case, IMHO). Line 129: } Line 130: Line 131: return getName().getIsValid() Line 132: && getCidr().getIsValid() http://gerrit.ovirt.org/#/c/25359/1/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/provider/DnsServersWidget.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/provider/DnsServersWidget.java: Line 34: Line 35: @Override Line 36: protected boolean isGhost(EntityModel<String> value) { Line 37: String text = value.getEntity(); Line 38: return text == null || text.isEmpty(); > Please use StringHelper.isNullOrEmpty() Done Line 39: } Line 40: Line 41: @Override Line 42: protected void toggleGhost(EntityModel<String> value, DnsServerEditor widget, boolean becomingGhost) { -- To view, visit http://gerrit.ovirt.org/25359 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c958328e769badebe2d95359d3ae81e657205c2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
