Zhou Zheng Sheng has posted comments on this change.
Change subject: pep8: fix E125
......................................................................
Patch Set 1: Verified; Looks good to me, but someone else must approve
(2 inline comments)
Run autobuild.sh OK. pydiff says
2 difference(s)
first file: vdsm/storage/volume.py
second file: /tmp/pydiff/vdsm/storage/volume.py
((833,
"For(AssName('v', 'OP_ASSIGN'), Name('vols'),
Stmt([If([(Compare(CallFunc(Getattr(CallFunc(Getattr(CallFunc(Getattr(Name('sdCache'),
'produce'), [Getattr(Name('self'), 'sdUUID')], None, None), 'produceVolume'),
[Getattr(Name('self'), 'imgUUID'), Name('v')], None, None), 'getParent'), [],
None, None), [('==', Getattr(Name('self'), 'volUUID'))]),
Stmt([Discard(CallFunc(Getattr(Name('children'), 'append'), [Name('v')], None,
None))]))], None)]), None)"),
(833,
"Assign([AssName('dom', 'OP_ASSIGN')], CallFunc(Getattr(Name('sdCache'),
'produce'), [Getattr(Name('self'), 'sdUUID')], None, None))"))
(('833 +', None),
(834,
For(AssName('v', 'OP_ASSIGN'), Name('vols'),
Stmt([If([(Compare(CallFunc(Getattr(CallFunc(Getattr(Name('dom'),
'produceVolume'), [Getattr(Name('self'), 'imgUUID'), Name('v')], None, None),
'getParent'), [], None, None), [('==', Getattr(Name('self'), 'volUUID'))]),
Stmt([Discard(CallFunc(Getattr(Name('children'), 'append'), [Name('v')], None,
None))]))], None)]), None)))
This is about extracting a common expression outside of the loop to make the
line shorter. It's OK.
....................................................
File vdsm/libvirtvm.py
Line 1342: for volInfo in drive.volumeChain:
Line 1343: if ('leasePath' not in volInfo or
Line 1344: 'leaseOffset' not in volInfo or
Line 1345: volInfo['shared']):
Line 1346: continue
I was struggling which style is better.
if ('leasePath' not in volInfo or
'leaseOffset' not in volInfo or
volInfo['shared']):
continue
or
if ('leasePath' not in volInfo or
'leaseOffset' not in volInfo or
volInfo['shared']):
continue
Both can be accepted by pep8. At last I think both are OK...
Line 1347:
Line 1348: leaseElem = self._buildLease(
Line 1349: drive.domainID, volInfo['volumeID'],
volInfo['leasePath'],
Line 1350: volInfo['leaseOffset'])
....................................................
File vdsm/storage/blockSD.py
Line 168: if vImg not in res[vName]['imgs']:
Line 169: res[vName]['imgs'].insert(0, vImg)
Line 170: if vPar != sd.BLANK_UUID and \
Line 171: not vName.startswith(sd.REMOVED_IMAGE_PREFIX) and \
Line 172: vImg not in res[vPar]['imgs']:
Though it's not related to this patch, the backslashes here are not nice. The
condition expression can be wrapped in parenthesis, and remove the backslash.
Line 173: res[vPar]['imgs'].append(vImg)
Line 174:
Line 175: return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent']))
Line 176: for k, v in res.iteritems())
--
To view, visit http://gerrit.ovirt.org/9805
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I63312573beac2dfd2fa3062c5e6656d8b7fb50ce
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
