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