Vinzenz Feenstra has posted comments on this change.

Change subject: Improve dom xml gereration
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

All in all this is very nice.
Just the minority issue of the one liner on line 599.

But I would prefer to see this change being applied after we implemented the 
unit tests for the XML generation. So that we're able verifying that there have 
been not added any regressions.

....................................................
File vdsm/libvirtvm.py
Line 595:         self.log = log
Line 596: 
Line 597:         self.doc = xml.dom.minidom.Document()
Line 598:         domType = ('kvm' if utils.tobool(self.conf.get('kvmEnable', 
'true'))
Line 599:                    else 'qemu')
I would prefer this not to be a one liner, the fact that you need to break it 
into two lines doesn't really help with the readability.
Line 600:         self.dom = XMLElem('domain', attrs={'type': domType})
Line 601:         self.doc.appendChild(self.dom)
Line 602: 
Line 603:         self.dom.addChild('name', text=self.conf['vmName'])


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib936740fcbeeee551e13abfed7fd91f2a159e351
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shao...@linux.vnet.ibm.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to