Zhou Zheng Sheng has posted comments on this change.
Change subject: Make domainMonitor compliant to PEP8 ......................................................................
Patch Set 17: Verified
(1 inline comment)
Passes tests in vdsm.
pydiff found some changes. After examine the corresponding lines, I think the changes are acceptable.
pydiff detailed report: pydiff domainMonitor*.py 4 difference(s) first file: domainMonitoro.py second file: domainMonitor.py
((73, 'st'), (78, 'status'))
((77, 'st'), (81, 'status'))
((80, 'st'), (85, 'status'))
((130, "Subscript(Dict([(Name('True'), Const('valid')), (Name('False'), Const('Invalid'))]), 'OP_APPLY', [Getattr(Name('nextStatus'), 'valid')])"), (137, "IfExp(Getattr(Name('nextStatus'), 'valid'), Const('Valid'), Const('Invalid'))"))
The first 3 are changing a variable name. It's a local variable, and all occurrences are changed. The last change is acceptable as well if you look at the code.
.................................................... File vdsm/storage/domainMonitor.py Line 31: "vgMdFreeBelowThreashold") PEP 8 says "Continuation lines should align wrapped elements either vertically using Python's implicit line joining inside parentheses, brackets and braces, or using a hanging indent." http://www.python.org/dev/peps/pep-0008/#indentation
So it would be nice here to write: __slots__ = ("error", "lastCheck", "valid", "readDelay", "masterMounted", "masterValid", "diskUtilization", "vgMdUtilization", "vgMdHasEnoughFreeSpace", "vgMdFreeBelowThreashold")
-- To view, visit http://gerrit.ovirt.org/3928 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie707eed5a132704378f0fd61f57e84e527fbc06d Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com