Dima Kuznetsov has posted comments on this change.

Change subject: tool: Make configurators more Pythonic
......................................................................


Patch Set 1:

(2 comments)

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

Line 51: 
Line 52: class Libvirt(ModuleConfigure):
Line 53: 
Line 54:     name = 'libvirt'
Line 55:     requires = {'certificates'}
I think set() is less confusing to write this, also, it returns a mutable object
Line 56:     services = ("vdsmd", "supervdsmd", "libvirtd")
Line 57: 
Line 58:     def _getFile(self, fname):
Line 59:         return self.FILES[fname]['path']


http://gerrit.ovirt.org/#/c/31741/1/tests/toolTests.py
File tests/toolTests.py:

Line 43: 
Line 44:     def __repr__(self):
Line 45:         return "name: %s, dependencies: %s" % (self._name, 
self._dependencies)
Line 46: 
Line 47:     @property
Maybe just assign to self.name in __init__() ?
Line 48:     def name(self):
Line 49:         return self._name
Line 50: 
Line 51:     @property


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16092da09d6763a8222bc941bd7d1501a7bb3bff
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Nir Soffer <[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