Nir Soffer has posted comments on this change. Change subject: securable: refactor the securable implementation ......................................................................
Patch Set 5: (1 comment) http://gerrit.ovirt.org/#/c/22115/5/vdsm/storage/securable.py File vdsm/storage/securable.py: Line 27: class SecureError(RuntimeError): Line 28: pass Line 29: Line 30: Line 31: def Secured(check_method_name): > I liked @secured better. It was capitalized to satisfy this request: This was not my intention. The problem was that having two public decorators, one called "secured" and one "unsecured". One would expect that both are class decorators or method decorators. If we rename to: @secured_class class Foo: @unsecured_method def somthing(self): ... There is no confusion. But I'm not that happy with those names. +1 for renaming it to "secured". Adding module documentation that explain how do you use those decorators will solve the possible confusion. For example, see monkeypatch.py. Line 32: def wrapper(cls): Line 33: for name, value in cls.__dict__.iteritems(): Line 34: if (not callable(value) or not getattr(value, SECURE_FIELD, True) Line 35: or name.startswith("__") # skipping builtins -- To view, visit http://gerrit.ovirt.org/22115 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
