Change in vdsm[master]: MonkeyPatch: fix reverting staticmethods.

danken at redhat.com danken at redhat.com
Mon Jan 20 09:57:21 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: MonkeyPatch: fix reverting staticmethods.
......................................................................


Patch Set 2:

(1 comment)

BTW, there is a parallel implementation of MonkeyPatch with the very same bug:

 https://code.launchpad.net/~danken/python-fixtures/staticmethod/+merge/202207

http://gerrit.ovirt.org/#/c/23378/2/tests/monkeypatch.py
File tests/monkeypatch.py:

Line 72:             # sets the function as instancemethod instead of keeping it as
Line 73:             # staticmethod.
Line 74:             if type(that) != type(getattr(module, name)):
Line 75:                 if isinstance(that, types.FunctionType):
Line 76:                     setattr(module, name, staticmethod(that))
> I suggest less by version tests if it can be concluded from state.
Anything that isn't outright equality is surprising, to say the least...

In general "it's better to ask for forgiveness" after a problem is found. But in this condition, when the fix fits to a very specific circumstances, I think it safer to test for it, and act only when it makes sense.
Line 77: 
Line 78: 
Line 79: #
Line 80: # Monkey patch scope.


-- 
To view, visit http://gerrit.ovirt.org/23378
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3100455ab761481f57a93ab8aacc34ac4ab91eb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list