Igor Lvovsky has posted comments on this change.

Change subject: [WIP] Remove subChainSizeCalc() and getSubChain(). Remove 
getAllChildrenList().
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(4 inline comments)

....................................................
File vdsm/storage/image.py
Line 1034:         volsImgs = dict((k, v) for k, v in allVols.iteritems() if 
imgUUID == v[0])
Edu, please use normal variable names, not 'k' and 'v'.
I know that it's looks very trivial for you but not for us (at least not for 
me).
I don't want to recall every time how getAllVolumes() output looks like.

Line 1068: 
Ah, I don't know. You wanted to get rid from subChainSizeCalc and getSubChain.
and you just put their code inline. As the result we got a huge merge function.

Line 1081:                 chainToRemove = self._baseRawVolumeMerge(sdDom, 
srcVolParams, volParams, [vols[vUUID] for vUUID in chain])
1. What is vols[vUUID]? is it images? why?
2. split the lines, please

Line 1084:                 chainToRemove = self._baseCowVolumeMerge(sdDom, 
srcVolParams, volParams, chainSize, chain)
again, split the lines

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffbbf71269eddb53c032c381d10e20b771c47c6b
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to