Francesco Romani has posted comments on this change.

Change subject: Introduction for caching the parsed domain XML
......................................................................


Patch Set 10:

(2 comments)

just a couple of nits. I'm really looking forward to have this merged!

http://gerrit.ovirt.org/#/c/17694/10/vdsm/vm.py
File vdsm/vm.py:

Line 464: 
Line 465:     def set(self, xml):
Line 466:         self._xml = xml
Line 467:         self._dom = _domParseStr(xml)
Line 468:         devices = self.firstElementByTagName('devices')
Unneeded temporary?
Line 469:         self._devices = devices
Line 470: 
Line 471:     def xml(self):
Line 472:         return self._xml


Line 476: 
Line 477:     def firstElementByTagName(self, tagName):
Line 478:         return 
self._dom.childNodes[0].getElementsByTagName(tagName)[0]
Line 479: 
Line 480:     def devs(self):
nit: 'devices' is better IMO.
Also wondering if we make a property of this one, 'dom' and 'xml'. This will 
lead to clearer code and save us a temporary (line 3281) and a few parens here 
and there.
Line 481:         return self._devices
Line 482: 
Line 483:     def getDeviceElements(self, tagName):
Line 484:         return self._devices.getElementsByTagName(tagName)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e106b2f2d3f4160d4e882f1a2880cb1b52fbb22
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to