Marcin Mirecki has posted comments on this change. Change subject: Replacing minidom with ElementTree in caps.py ......................................................................
Patch Set 5: (1 comment) The choice is between using either -iter().next() #finds all subelements recursively -find() and an xpath query #finds what the query specifies (this is what ElementTree offers us) The iter() function seemed more readable to me, so I used this one. The differences are slight, so I am open to change if you find the other way more concise. https://gerrit.ovirt.org/#/c/41078/5/debian/vdsm-tests.install File debian/vdsm-tests.install: Line 7: usr/share/vdsm/tests/caps_libvirt_intel_i73770.out Line 8: usr/share/vdsm/tests/caps_libvirt_intel_i73770_nosnap.out Line 9: usr/share/vsdm/tests/caps_numactl_4_nodes.out Line 10: usr/share/vdsm/tests/cpu_info.out Line 11: usr/share/vdsm/tests/cpu_map.xml > I don't see this file added (moreover, is that relevant for a refactor? If The file was added in the previous patch: https://gerrit.ovirt.org/#/c/41077 The changes were split into two patches. One added the unit tests without any refactoring, the second was refactoring of the code. The previous patch would actually be a better place for this change. Line 12: usr/share/vdsm/tests/devices/parsing*.py Line 13: usr/share/vdsm/tests/devices/data/*.xml Line 14: usr/share/vdsm/tests/functional/*.policy Line 15: usr/share/vdsm/tests/functional/*.py -- To view, visit https://gerrit.ovirt.org/41078 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c1ca58807515922347e2255c77b4f950decd619 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
