Change in vdsm[master]: clientIF: prepareVolumePath payload cleanup

nsoffer at redhat.com nsoffer at redhat.com
Mon Feb 17 17:25:19 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: clientIF: prepareVolumePath payload cleanup
......................................................................


Patch Set 7:

- The new code looks clean and good
- The commit message is confusing, as this is a refactoring and it does not fix any bug - we can replace the Bug-Url with Relates-To, since the bug explains why the refactoring is needed.
- The test module is a great addition, but has many minor issues which make it harder to understand and use for adding more tests. I talked with Francesco about all the issues and hopefully he will address them in the next patch.
- An important test is missing, checking that we do reach the old code when a drive does not have a specParams key (the tests test only an empty specParams dict).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a630d74ec0910c669e0326ad343c5dbea25357e
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No


More information about the vdsm-patches mailing list