Dan Kenigsberg has posted comments on this change. Change subject: netinfo: fix getMaxMtu generator handling ......................................................................
Patch Set 1: I would prefer that you didn't submit this (2 inline comments) Thanks for this catch, Toni! But clearer attribution is important imo. .................................................... Commit Message Line 7: netinfo: fix getMaxMtu generator handling Line 8: Line 9: getMaxMtu was changed to use iterables instead of just lists. Line 10: The check for if and a generator is always True regardless of Line 11: it generating any elements. Thus, max could fail. Mentioning that this regression was introduced by http://gerrit.ovirt.org/15355 may teach its authors and reviewers to pay more attention! Line 12: Line 13: Change-Id: I94a58ab6a3303099105987fd230680586f6a3cce .................................................... File lib/vdsm/netinfo.py Line 143: Line 144: getMaxMtu return the highest value in a connection tree, Line 145: it check if a vlan, bond that have a higher mtu value Line 146: """ Line 147: devs_mtu = [getMtu(dev) for dev in devs] Frankly, I find return max([getMtu(dev) for dev in devs] + [mtu]) clearer (but maybe that's only me) Line 148: if devs_mtu: Line 149: return max(mtu, *devs_mtu) Line 150: else: Line 151: return mtu -- To view, visit http://gerrit.ovirt.org/15903 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I94a58ab6a3303099105987fd230680586f6a3cce Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
