)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f4899ad93baf8c8478870be9eca4d9d5b0092de4","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added appender methods to remove clones from config.py"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Reducing source code lines by extracting duplicated code blocks."},{"line_number":10,"context_line":"The clone metrics before and after can be checked here:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"http://openqa.sed.hu/dashboard/index/4?did\u003d4\u0026period\u003d2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1aa78d24_be1273b2","line":9,"updated":"2016-07-05 08:26:12.000000000","message":"So for me this is the wrong way around.\n\nReducing repetition and keeping the code DRY is a worthy aim, reducing the number of source code lines doesn\u0027t matter.","commit_id":"3f77cef0b375abf2d76fbd6266736be4f226059a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"29d20c5df398611b3776feb1be470cc462b26078","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove duplicated code blocks in format_dom"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Extracting duplicated code blocks into some helper methods to help keep"},{"line_number":10,"context_line":"the code DRY."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I975fabf92121129aca46c0e56399638b3d14950f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a8ec9b2_76d698c4","line":10,"updated":"2016-09-20 11:35:18.000000000","message":"Please avoid obscure acronyms like \"DRY\" - explain what you\u0027re doing in plain english","commit_id":"a502ec8e7f968d8485939d305f394be518833104"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"061c1e956cbcff88b1b5cd50ff7be1a2daaa0ed1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove duplicated code blocks in format_dom"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Extracting duplicated code blocks into some helper methods to help keep"},{"line_number":10,"context_line":"the code DRY."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I975fabf92121129aca46c0e56399638b3d14950f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a8ec9b2_79e20b03","line":10,"in_reply_to":"7a8ec9b2_76d698c4","updated":"2016-09-20 11:45:07.000000000","message":"That was added by John Garbutt. Please, tell him your opinion about it. However, this commit message is outdated. I\u0027ll update it soon. Thanks for your review!","commit_id":"a502ec8e7f968d8485939d305f394be518833104"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8c139d77eb6ed60039f968e6c5bb3d5b4625e18f","unresolved":false,"context_lines":[{"line_number":630,"context_line":"        sysinfo.set(\"type\", self.type)"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"        bios \u003d [\"bios\", None]"},{"line_number":633,"context_line":"        system \u003d [\"system\", None]"},{"line_number":634,"context_line":""},{"line_number":635,"context_line":"        def info_appender(where_to_append, element_to_append, element_name):"},{"line_number":636,"context_line":"            if element_to_append is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_8950230c","line":633,"updated":"2016-07-05 08:41:52.000000000","message":"This is silly just do\n\n  bios \u003d etree.Element(\"bios\")\n\nand at the end do\n\n  if len(bios.get_children) \u003e 0:\n     sysinfo.append(bios)","commit_id":"54f6ffbea60b6306d484cc0267f2594356d6359f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8c139d77eb6ed60039f968e6c5bb3d5b4625e18f","unresolved":false,"context_lines":[{"line_number":639,"context_line":"                info \u003d etree.Element(\"entry\", name\u003delement_name)"},{"line_number":640,"context_line":"                info.text \u003d element_to_append"},{"line_number":641,"context_line":"                where_to_append[1].append(info)"},{"line_number":642,"context_line":""},{"line_number":643,"context_line":"        info_appender(bios, self.bios_vendor, \"vendor\")"},{"line_number":644,"context_line":"        info_appender(bios, self.bios_version, \"version\")"},{"line_number":645,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_09f553ca","line":642,"updated":"2016-07-05 08:41:52.000000000","message":"IMHO, we should just use the existing _text_node() method on the base class, as it simplifies the code while still making it clear what the logic is.","commit_id":"54f6ffbea60b6306d484cc0267f2594356d6359f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8c139d77eb6ed60039f968e6c5bb3d5b4625e18f","unresolved":false,"context_lines":[{"line_number":640,"context_line":"                info.text \u003d element_to_append"},{"line_number":641,"context_line":"                where_to_append[1].append(info)"},{"line_number":642,"context_line":""},{"line_number":643,"context_line":"        info_appender(bios, self.bios_vendor, \"vendor\")"},{"line_number":644,"context_line":"        info_appender(bios, self.bios_version, \"version\")"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"        info_appender(system, self.system_manufacturer, \"manufacturer\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_29212f4e","line":643,"updated":"2016-07-05 08:41:52.000000000","message":"eg,\n\n     if self.bios_vendor is not None:\n         bios.append(self._text_node(\"entry\", self.bios_vendor, name\u003d\"manufacturer\"))","commit_id":"54f6ffbea60b6306d484cc0267f2594356d6359f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8c139d77eb6ed60039f968e6c5bb3d5b4625e18f","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        def append_iotune(element_to_append, element_string):"},{"line_number":764,"context_line":"            if element_to_append is not None:"},{"line_number":765,"context_line":"                iotune.append(self._text_node(element_string,"},{"line_number":766,"context_line":"                                              element_to_append))"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        append_iotune(self.disk_read_bytes_sec, \"read_bytes_sec\")"},{"line_number":769,"context_line":"        append_iotune(self.disk_read_iops_sec, \"read_iops_sec\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_89a4a3b9","line":766,"updated":"2016-07-05 08:41:52.000000000","message":"IMHO this is just a pointless helper, particularly when you start duplicating this in every format_dom method, with slightly different semantics in each. IMHO there\u0027s nothing that needs fixing in this method - its just bikeshedding on style, and making this code needlessly different from other methods in this file.","commit_id":"54f6ffbea60b6306d484cc0267f2594356d6359f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"29d20c5df398611b3776feb1be470cc462b26078","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                 **kwargs)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _text_node(self, node_name, value, **kwargs):"},{"line_number":60,"context_line":"        child \u003d self._new_node(node_name, **kwargs)"},{"line_number":61,"context_line":"        child.text \u003d six.text_type(value)"},{"line_number":62,"context_line":"        return child"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_168394cd","line":60,"updated":"2016-09-20 11:35:18.000000000","message":"Please remove this pointless renaming","commit_id":"a502ec8e7f968d8485939d305f394be518833104"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"061c1e956cbcff88b1b5cd50ff7be1a2daaa0ed1","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                 **kwargs)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _text_node(self, node_name, value, **kwargs):"},{"line_number":60,"context_line":"        child \u003d self._new_node(node_name, **kwargs)"},{"line_number":61,"context_line":"        child.text \u003d six.text_type(value)"},{"line_number":62,"context_line":"        return child"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_f9845b5e","line":60,"in_reply_to":"7a8ec9b2_168394cd","updated":"2016-09-20 11:45:07.000000000","message":"It\u0027s not pointless. As the text node should has the name attribute (or so), but if you try to add an attribute with \"name\" in kwargs, that would make an error as name would have been in there two times: the first parametes has the name \"name\" and it would be also in the kwargs. So that\u0027s the reason for renaming. Also, in this way it is more clearer which is node_name for.","commit_id":"a502ec8e7f968d8485939d305f394be518833104"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1182398b9308e59ef6686c9ea25496e3e9e7637a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                 **kwargs)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _text_node(self, node_name, value, **kwargs):"},{"line_number":60,"context_line":"        child \u003d self._new_node(node_name, **kwargs)"},{"line_number":61,"context_line":"        child.text \u003d six.text_type(value)"},{"line_number":62,"context_line":"        return child"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_39eb237a","line":60,"in_reply_to":"7a8ec9b2_f9845b5e","updated":"2016-09-20 11:50:47.000000000","message":"Ok, please do this in a separate commit from the rest, and add test coverage for that scenario","commit_id":"a502ec8e7f968d8485939d305f394be518833104"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"29d20c5df398611b3776feb1be470cc462b26078","unresolved":false,"context_lines":[{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        if self.disk_read_bytes_sec is not None:"},{"line_number":777,"context_line":"            iotune.append(self._text_node(\"read_bytes_sec\","},{"line_number":778,"context_line":"                                          self.disk_read_bytes_sec))"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        if self.disk_read_iops_sec is not None:"},{"line_number":781,"context_line":"            iotune.append(self._text_node(\"read_iops_sec\","}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_d6112cb1","line":778,"updated":"2016-09-20 11:35:18.000000000","message":"Don\u0027t add arbitrary whitespace changes to commits.","commit_id":"a502ec8e7f968d8485939d305f394be518833104"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"061c1e956cbcff88b1b5cd50ff7be1a2daaa0ed1","unresolved":false,"context_lines":[{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        if self.disk_read_bytes_sec is not None:"},{"line_number":777,"context_line":"            iotune.append(self._text_node(\"read_bytes_sec\","},{"line_number":778,"context_line":"                                          self.disk_read_bytes_sec))"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        if self.disk_read_iops_sec is not None:"},{"line_number":781,"context_line":"            iotune.append(self._text_node(\"read_iops_sec\","}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_d9cf9fb1","line":778,"in_reply_to":"7a8ec9b2_d6112cb1","updated":"2016-09-20 11:45:07.000000000","message":"That\u0027s becouse of the continous line indentation requires this, however, you\u0027re right. maybe it would be nice in another patchset.","commit_id":"a502ec8e7f968d8485939d305f394be518833104"}]}