Martin Mucha has posted comments on this change. Change subject: core: refactored RoleTreeView for reababilily purposes. ......................................................................
Patch Set 26: (1 comment) http://gerrit.ovirt.org/#/c/29899/26/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java: Line 53: Line 54: return roleTreeView; Line 55: } Line 56: Line 57: private static RoleNode categoryNode(String name, RoleNode... leaves) { > I think this method is called well - if you look at the usage it is obvious My main (and sole) intention is to improve readability. we can view it from two perspectives: a) "roleNode(...)" represents role node. b) describing function and it's purpose, and then createRoleNode would be better. I don't care about naming, however we're little bit short of free space on each line, so adding 5 letters isn't that cheap. But I have no problem with it. new methods are static factories, evidently. I wasn't sure if RoleNode is used only like this, so I did not publish them in RoleNode. If that will be ok, making them public on RoleNode would be better. —— so what's the concensus here? with "create" prefix and on RoleNode class, or ... there are at least four combinations... Line 58: return new RoleNode(name, leaves); Line 59: } Line 60: Line 61: private static RoleNode categoryNode(String name, String tooltip, RoleNode... leaves) { -- To view, visit http://gerrit.ovirt.org/29899 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I294e37d07a52c894e4293ca54f9cdb1003df8913 Gerrit-PatchSet: 26 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
