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