Dan Kenigsberg has posted comments on this change. Change subject: configNetworkTests: use context to manage monkey patches ......................................................................
Patch Set 5: I would prefer that you didn't submit this (2 inline comments) thanks, that's a cool decorator! (but I think it is slightly buggy) .................................................... File tests/monkeypatch.py Line 38: def decorator(f): I think you should use @functools.wraps so that the wrapping function has the same name as the wrapped one. Line 42: f(*args, **kw) do we really want to swallow the returned value of f()? I'd return it. But worse: you do not protect here from exceptions. Cannot you use the contextmanager that you've defined below? -- To view, visit http://gerrit.ovirt.org/6939 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ecb4589f2edbf605e62a8963889d314c2535537 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Gal Hammer <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Livnat Peer <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
