Zhou Zheng Sheng has posted comments on this change.

Change subject: Fix Fedora 18 pyflakes warning for lvm.py
......................................................................


Patch Set 1:

Thanks Eduardo and Douglas Schilling Landgraf. I have see the bug link in 
http://gerrit.ovirt.org/#/c/12700/2 . For this pyflakes bug, there a 5 options 
currently.

 A: downgrade pyflakes on the Jenkins slave;
 B: put lvm.py into PYFLAKES_BLACKLIST;
 C: do nothing and wait till pyflakes is fixed;
 D: try to just ignor this kind of warning;
 E: edit the list comprehension variable name lv in lvm.py.

A and B will stop pyflakes from complaining, but reduce the checking coverage. 
If there are other kind of errors in the future patch for lvm.py, it may not be 
covered.

C is ostrichism. Since we are applying Jenkins to run test and check for newly 
submitted patches, all the patches will be with this pyflakes warning, making 
the good and bad Jenkins test report undistinguishable.

D is not very nice either, it will ignore the following case as well.

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

Run it in python, it prints 3. It seems Python 2.7 is leaking list 
comprehension temp variable. This is error prone and I think we should not skip 
this kind of warning.

E is compromise and makes the code verbose. I personally can accept this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb7d7285a8045748f02fb9096d8622888892d8d0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Eduardo <[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