Lior Vernia has posted comments on this change. Change subject: webadmin: Add common MAC pool widget ......................................................................
Patch Set 11: (3 comments) http://gerrit.ovirt.org/#/c/27790/11/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacPoolModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacPoolModel.java: Line 58: Collections.sort(rangeModels, new Comparator<MacRangeModel>() { Line 59: Line 60: @Override Line 61: public int compare(MacRangeModel range1, MacRangeModel range2) { Line 62: return range1.getLeftBound().getEntity().compareTo(range2.getRightBound().getEntity()); > Can you please explain this sort? I don't really understand it... Indeed not very clear what this was supposed to do :) Line 63: } Line 64: }); Line 65: macRanges.setItems(rangeModels); Line 66: } http://gerrit.ovirt.org/#/c/27790/11/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacRangeModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacRangeModel.java: Line 46: public boolean validate() { Line 47: leftBound.validateEntity(new IValidation[] { new MacAddressValidation() }); Line 48: rightBound.validateEntity(new IValidation[] { new MacAddressValidation() }); Line 49: if (leftBound.getIsValid() && rightBound.getIsValid()) { Line 50: rightBound.validateEntity(new IValidation[] { new IValidation() { > Please extract the validator to a separate class. Doesn't seem likely that this will be reused somewhere, but done. Line 51: Line 52: @Override Line 53: public ValidationResult validate(Object value) { Line 54: ValidationResult res = new ValidationResult(); http://gerrit.ovirt.org/#/c/27790/11/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/macpool/MacRangeEditor.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/macpool/MacRangeEditor.java: Line 64: Line 65: @Override Line 66: public void edit(final MacRangeModel model) { Line 67: driver.edit(model); Line 68: leftBound.addKeyDownHandler(new KeyDownHandler() { > This is not the first time I see that KeyDown triggers ValueChagnedEvent. Done, see newly-added preceding patch. Line 69: Line 70: @Override Line 71: public void onKeyDown(final KeyDownEvent event) { Line 72: Scheduler.get().scheduleDeferred(new ScheduledCommand() { -- To view, visit http://gerrit.ovirt.org/27790 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ife55cf7c5e005912639c087565c3c5c3faddd93d Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[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
