Zhou Zheng Sheng has posted comments on this change.

Change subject: Makefile: lvm.py into PYFLAKES_BLACKLIST
......................................................................


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

Though the warning message from pyflakes is the same as in the bug link, it's 
not the same bug.

Firstly there is a python 2.7 bug. List comprehension will leak temp variable. 
For example.

 t = 20
 x = [t for t in [1, 2, 3]]
 print t

The result is 3. This bug is fixed in python 3. dict and set will not leak temp 
variable in generator, dict and set comprehension. The aforementioned pyflakes 
bug is reporting variable redefinition in all the cases. Now the pyflakes is 
fixed and only report variable redefinition for list comprehension. This is all 
the bug link about.

lvm.py triggers a different pyflakes bug. As mentioned by Eduardo, "lv was not 
instatiated yet if you reach the else. There are two different lv definition in 
if - else clauses which are mutually excluyent." The pyflakes is not smart 
enough to detect the variable "lv" in "else" block in list comprehension is a 
different variable from the "lv" in the "if" block.

I think put lvm.py into PYFLAKES_BLACKLIST will reduce the check coverage. If 
there are defects in future lvm.py patches, we will not be able to detect them. 
I think for now we can just downgrade the pyflakes for a while, or just make a 
compromise and change the variable name in the list comprehension...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8cdeb0c28c1ee5dc3a18a7688ae60fe641f9a6c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[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

Reply via email to