Saggi Mizrahi has posted comments on this change.

Change subject: vdsm-tool: reorgenize module configurers.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/31400/2/lib/vdsm/tool/configurators/libvirt.py
File lib/vdsm/tool/configurators/libvirt.py:

Line 50: if utils.isOvirtNode():
Line 51:     from ovirt.node.utils.fs import Config as NodeCfg
Line 52: 
Line 53: 
Line 54: class Libvirt(ModuleConfigure):
The inheritance here is meaningless.
You can see it is by the fact that you haven't initialized the base class.

In any case, what you want is an interface and since those are implicit in 
python I'd just avoid creating needless classes and complications in the type 
system.
Line 55: 
Line 56:     def getName(self):
Line 57:         return 'libvirt'
Line 58: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic79d400760e425f6f4863b6a33c69aa14fb0c623
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to