Martin Sivák has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. ......................................................................
Patch Set 1: Looks good to me, but someone else must approve (1 inline comment) This looks good, except for one style comment. .................................................... File vdsm/netconf/ifcfg.py Line 695: getMaxMtu return the highest value in a connection tree, Line 696: it check if a vlan, bond that have a higher mtu value Line 697: """ Line 698: Line 699: return max(mtu, *[int(netinfo.getMtu(nic)) for nic in nics]) Can you please split this into two lines? Save the list of mtus and then call max() on it. Although this is correct, it is not easy to read. Line 700: Line 701: def setNewMtu(self, network, bridged, _netinfo=None): Line 702: """ Line 703: Set new MTU value to network and its interfaces -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
