Ido Barkan has posted comments on this change. Change subject: network: api: remove bond objectivization duplicated code ......................................................................
Patch Set 2: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/37750/2/vdsm/network/api.py File vdsm/network/api.py: Line 596: else: Line 597: addition.append((name, attrs)) Line 598: Line 599: def _getBondObject(): Line 600: bond = Bond.objectivize(name, configurator, attrs.get('options'), > I cannot say that I'm mad for this patch: this is a single function call (w Ido thinks that there is more repeated code to remove. the exact same code is executed under the previous 'for' statement that should be also factored out. and regardless, I would prefer that the code looked something like: for name, attrs in bondings.items(): # this is done for all bonds regardless bond = Bond.objectivize(...params...) # now add/remove/edit the bond IMO this is more concise and readable. now you won't have to factor out the objectivize() call Line 601: attrs.get('nics'), mtu=None, _netinfo=_netinfo, Line 602: destroyOnMasterRemoval='remove' in attrs) Line 603: return bond Line 604: -- To view, visit http://gerrit.ovirt.org/37750 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0dd75fa37a687dcd9b32519e075cf3efc7aa08e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
