Hello ShaoHe Feng,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
make libvirtev.py PEP8 clean
Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Signed-off-by: ShaoHe Feng shaohef@linux.vnet.ibm.com --- M Makefile.am M vdsm/libvirtev.py 2 files changed, 90 insertions(+), 55 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/20/5420/1 -- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com
Bing Bu Cao has posted comments on this change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
Patch Set 1: Verified
Passes tests in vdsm.
pydiff find some changes, but all of them are deleting useless ";" after a statement. For example, delete the ";" in "h.set_interval(interval);". The changes are acceptable.
pydiff detailed report: pydiff libvirtev*.py 19 difference(s) first file: libvirtevo.py second file: libvirtev.py
((292, Discard(Const(None))), (292, None))
((327, Discard(Const(None))), (327, None))
((329, Discard(Const(None))), (329, None))
((334, 'Discard(Const(None))'), (335, "If([(Bitand([Name('events'), Getattr(Name('select'), 'POLLIN')]), Stmt([AugAssign(Name('ret'), '|=', Getattr(Name('libvirt'), 'VIR_EVENT_HANDLE_READABLE'))]))], None)"))
((335, 'POLLIN'), (337, 'POLLOUT'))
((336, 'VIR_EVENT_HANDLE_READABLE'), (338, 'VIR_EVENT_HANDLE_WRITABLE'))
((336, 'Const(None)'), (338, "Getattr(Name('libvirt'), 'VIR_EVENT_HANDLE_WRITABLE')"))
((337, 'POLLOUT'), (339, 'POLLNVAL'))
((338, 'VIR_EVENT_HANDLE_WRITABLE'), (340, 'VIR_EVENT_HANDLE_ERROR'))
((338, 'Const(None)'), (340, "Getattr(Name('libvirt'), 'VIR_EVENT_HANDLE_ERROR')"))
((339, 'POLLNVAL'), (341, 'POLLERR'))
((340, Discard(Const(None))), (342, None))
((341, 'POLLERR'), (343, 'POLLHUP'))
((342, 'VIR_EVENT_HANDLE_ERROR'), (344, 'VIR_EVENT_HANDLE_HANGUP'))
((342, 'Const(None)'), (344, "Getattr(Name('libvirt'), 'VIR_EVENT_HANDLE_HANGUP')"))
((344, Discard(Const(None))), (344, None))
((345, Discard(Const(None))), (345, None))
((429, Discard(Const(None))), (438, None))
((429, Discard(Const(None))), (438, None))
In the report, ";" is parsed as "Discard(Const(None))", so " ((292, Discard(Const(None))), (292, None))" means the patch deletes a ";" in line 292. Line numbers may be a little inaccurate, because the Python compiler module does not give accurate line number info on "Discard" nodes.
The changes detected in line 334 to 342 seems to be big at first glance. It's because the case is special, and cause the pydiff algorithm raise some false alarms. By inspecting these suspicious lines in the source, we can know they are caused by deleting ";" as well. The changes are acceptable.
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: make libvirtev.py PEP8 clean ......................................................................
Patch Set 1: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: make libvirtev.py PEP8 clean ......................................................................
make libvirtev.py PEP8 clean
Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Signed-off-by: ShaoHe Feng shaohef@linux.vnet.ibm.com --- M Makefile.am M vdsm/libvirtev.py 2 files changed, 90 insertions(+), 55 deletions(-)
Approvals: ShaoHe Feng: Looks good to me, but someone else must approve Bing Bu Cao: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Zhou Zheng Sheng: Verified; Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5420 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I8b9f3c1dfcbc2c2fe371a6e475f4d3f0ee72f650 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Bing Bu Cao mars@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org