Lior Vernia has posted comments on this change. Change subject: webadmin: Small refactoring to simplify VnicProfileModel ......................................................................
Patch Set 1: (2 comments) .................................................... Commit Message Line 7: webadmin: Small refactoring to simplify VnicProfileModel Line 8: Line 9: It wasn't really clear what exactly dcId and dcCompatibilityVersion Line 10: were affecting, whereas customPropertiesVisible was passed around as a Line 11: parameter when it really shoul only be set when the VnicProfileModel Fixed in next patchset I'll push. Line 12: is constructed. Line 13: Line 14: Also theoretically improved the behavior of selecting a QoS Line 15: configuration from the list. Currently when editing a network the DC .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileModel.java Line 353: getNetworkQoS().setSelectedItem(item); Line 354: return; Line 355: } Line 356: } Line 357: setSelectedNetworkQoSId(Guid.Empty); Well, as it's currently implemented I need to somehow find the item in the list that corresponds to the empty item, or create a new empty item. I conjecture that this is here to avoid duplicating the code creating an empty QoS item (see initNetworkQoSList()). That code could be extracted to another method, but then there's the weird thing of having two different empty QoS instances, one in the list and one that's the selected item, which aren't the same one. I think the best solution would be to add a static NetworkQoS.EMPTY member, and use it everywhere - this way there'll only be one empty QoS (it doesn't make sense to have multiple instances of an empty object), and its implementation will be encapsulated within the class itself rather than in models that use it (like this one). I don't mind fixing it as part of this patch, are you on board with the proposed solution? Line 358: } -- To view, visit http://gerrit.ovirt.org/20749 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e98a1fc26ce6b32076e09a67e6a3d82f52964df Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
