Change in vdsm[master]: Added VDSM interface for the virt-alignment-scan tool from l...

ewoud at kohlvanwijngaarden.nl ewoud at kohlvanwijngaarden.nl
Wed Mar 28 13:45:27 UTC 2012


Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: Added VDSM interface for the virt-alignment-scan tool from libguestfs
......................................................................


Patch Set 5: (5 inline comments)

Some inline comments. All mostly trivial style points, but gerrit brings out the perfectionist in me ;)

....................................................
File tests/alignmentScan_test.py
Line 29:     def runTest(self):
I think I'd prefer RunScanArgsTestCase as class name. That instantly shows what you're testing. You might name the test test_help_response.

Line 43:     assert(r == 0)
The parenthesis are not needed here

Line 48:         r, o, e = execCmd(cmd, data = "1,,\n")
I prefer a separate data variable:

 data = "128,,\n" if aligned else "1,,\n"
 r, o, e = execCmd(cmd, data = "1,,\n")
 assert r == 0

Line 52:         self.assertRaises(Exception, scanImage, ("nonexistent-image-name"))
You could test for the VirtAlignError instead of Exception

Line 65:             mkimage(img.name)
This line looks like a copy paste error

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d2c146a0fa3101317720f2e9c373e0d21b5cfdf
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saša Tomić <tomic80 at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud at kohlvanwijngaarden.nl>
Gerrit-Reviewer: Saša Tomić <tomic80 at gmail.com>


More information about the vdsm-patches mailing list