Change in vdsm[master]: virt: factor out _getUnderlying* methods to another class.

ibarkan at redhat.com ibarkan at redhat.com
Sun Mar 15 05:50:39 UTC 2015


Ido Barkan has posted comments on this change.

Change subject: virt: factor out _getUnderlying* methods to another class.
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/38704/2//COMMIT_MSG
Commit Message:

Line 19: a. Add unit tests for this class.
Line 20: b. The technique in which getUnderlyingVmDevicesInfo changes the 'conf'
Line 21:    reference instead of returning data is cumbersome and hard to read. It
Line 22:    should return an updated copy of of the conf dictionary to the caller.
Line 23:    it will ease testing and debugging.
> Right.
IMO, this is the farthest we can change any bit of logic here without adding unit tests. This whole class should be gone after it is unit tested (it's now easy. beforehand it was very hard). what you propose here is probably on the right track, but the fear of breaking things is the reason why it will never happen.
Line 24: 
Line 25: Change-Id: Ia04b6ee17da207f772f77059e45be97594e6be78


-- 
To view, visit https://gerrit.ovirt.org/38704
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia04b6ee17da207f772f77059e45be97594e6be78
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list