Change in vdsm[master]: vmDevices: add __slots__ to devices
nsoffer at redhat.com
nsoffer at redhat.com
Thu Feb 6 22:40:01 UTC 2014
Nir Soffer has posted comments on this change.
Change subject: vmDevices: add __slots__ to devices
......................................................................
Patch Set 5: Code-Review+1
(2 comments)
Minor cleanup would be nice.
http://gerrit.ovirt.org/#/c/21036/5/vdsm/vm.py
File vdsm/vm.py:
Line 1238: class VmDevice(object):
Line 1239: __slots__ = ('deviceType', 'device', 'alias', 'specParams', 'deviceId',
Line 1240: 'conf', 'log', '_deviceXML', 'type')
Line 1241:
Line 1242: def __init__(self, conf, log, **kwargs):
self.conf should be initialized here - initializing variables in argument order makes it easier to follow the code.
I wouldn't mention this if you leave self.log = log where it was, but since you moved it, self.conf should also move.
Line 1243: self.log = log
Line 1244: self.specParams = {}
Line 1245: for attr, value in kwargs.iteritems():
Line 1246: try:
Line 1247: setattr(self, attr, value)
Line 1248: except AttributeError: # skip read-only properties
Line 1249: self.log.debug('Ignoring param (%s, %s) in %s', attr, value,
Line 1250: self.__class__.__name__)
Line 1251: pass
pass is redundant, replaced by the log.
Line 1252: self.conf = conf
Line 1253: self._deviceXML = None
Line 1254:
Line 1255: def __str__(self):
--
To view, visit http://gerrit.ovirt.org/21036
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e8dadabdd02d3b44606f215c4bc7b7e306a591a
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpoledni at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek 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