)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4c8f583726420f19d46a8b2b4c7bb32d44f54a51","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Parse \u003cemulator\u003e elements from virConnectGetCapabilities()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ongoing / future work such as SEV[0] and the move of the default machine"},{"line_number":10,"context_line":"type to q35[1] will need to invoke virConnectGetDomainCapabilities()"},{"line_number":11,"context_line":"in order to check whether SEV or q35 respectively are supported."},{"line_number":12,"context_line":"However this API requires 5 parameters[2]:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_5384d95d","line":9,"updated":"2019-03-01 18:47:01.000000000","message":"Drive-by review here, but can we *start* with what this patch is doing. The explanations are great context and should definitely remain, but I\u0027d start with something like:\n\nThis patch adds an `emulator` field to the LibvirtConfigCapsGuest object corresponding to the arch/emulator element in https://libvirt.org/formatcaps.html#elementGuest. This will be used in the future to blah blah","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"66370ddd3beedc572d38274625bfa72bc95c1917","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Parse \u003cemulator\u003e elements from virConnectGetCapabilities()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ongoing / future work such as SEV[0] and the move of the default machine"},{"line_number":10,"context_line":"type to q35[1] will need to invoke virConnectGetDomainCapabilities()"},{"line_number":11,"context_line":"in order to check whether SEV or q35 respectively are supported."},{"line_number":12,"context_line":"However this API requires 5 parameters[2]:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_e344ec1b","line":9,"in_reply_to":"9fdfeff1_5384d95d","updated":"2019-03-05 10:09:46.000000000","message":"Done","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ddcac618463cedb96bb7d415a54589fe188a49fe","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Parse \u003cemulator\u003e elements from virConnectGetCapabilities()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ongoing / future work such as SEV[0] and the move of the default machine"},{"line_number":10,"context_line":"type to q35[1] will need to invoke virConnectGetDomainCapabilities()"},{"line_number":11,"context_line":"in order to check whether SEV or q35 respectively are supported."},{"line_number":12,"context_line":"However this API requires 5 parameters[2]:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_12ef2a1e","line":9,"in_reply_to":"9fdfeff1_5384d95d","updated":"2019-03-01 20:30:14.000000000","message":"Yeah, reasonable suggestion - currently there is quite a lot of context to wade through before getting to the change description.","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4c8f583726420f19d46a8b2b4c7bb32d44f54a51","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        # its own \u003cemulator\u003e sub-child."},{"line_number":309,"context_line":"        self.emulator \u003d None"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # Per-\u003cdomain\u003e overrides for the default in self.emulator"},{"line_number":312,"context_line":"        self.domemulator \u003d dict()"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def parse_dom(self, xmldoc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_73d55d42","line":311,"updated":"2019-03-01 18:47:01.000000000","message":"Is this a thing? I don\u0027t see it anywhere in https://libvirt.org/formatcaps.html\n\nIf it is, maybe make into a LibvirtConfigCapsGuestDomain object, because the nesting is getting pretty intense.","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"7ae5986bae993283995244cadf73466dec0dd07e","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        # its own \u003cemulator\u003e sub-child."},{"line_number":309,"context_line":"        self.emulator \u003d None"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # Per-\u003cdomain\u003e overrides for the default in self.emulator"},{"line_number":312,"context_line":"        self.domemulator \u003d dict()"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def parse_dom(self, xmldoc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_56b27008","line":311,"in_reply_to":"9fdfeff1_327fce67","updated":"2019-03-04 10:58:34.000000000","message":"I see both of your points.  Probably keep it as-is but call out that \u0027domemulator\u0027 is not an official libvirt XML attribute and describe a bit more why you\u0027re using it?  That gives more context to the next person who wonders about it.","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"1177ce0b2865a19cef892a41fec839f78bbc2dbe","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        # its own \u003cemulator\u003e sub-child."},{"line_number":309,"context_line":"        self.emulator \u003d None"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # Per-\u003cdomain\u003e overrides for the default in self.emulator"},{"line_number":312,"context_line":"        self.domemulator \u003d dict()"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def parse_dom(self, xmldoc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_bf8043f9","line":311,"in_reply_to":"9fdfeff1_56b27008","updated":"2019-03-04 19:40:53.000000000","message":"Done","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ddcac618463cedb96bb7d415a54589fe188a49fe","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        # its own \u003cemulator\u003e sub-child."},{"line_number":309,"context_line":"        self.emulator \u003d None"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # Per-\u003cdomain\u003e overrides for the default in self.emulator"},{"line_number":312,"context_line":"        self.domemulator \u003d dict()"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def parse_dom(self, xmldoc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_327fce67","line":311,"in_reply_to":"9fdfeff1_73d55d42","updated":"2019-03-01 20:30:14.000000000","message":"\u003e Is this a thing? I don\u0027t see it anywhere in https://libvirt.org/formatcaps.html\n\nIt\u0027s not well documented but it\u0027s clear when you look at some real-world XML, or even the test fixture in this review:\n\nhttps://review.openstack.org/#/c/640483/1/nova/tests/unit/virt/libvirt/test_config.py@119\n\n\u003e If it is, maybe make into a LibvirtConfigCapsGuestDomain object,\n\u003e because the nesting is getting pretty intense.\n\nYeah I thought about that but wondering if it was too heavyweight.  Let\u0027s see what others think.","commit_id":"52411867c391e04cbb227eb3bb6ef1af00e9358f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4c8b97fb6ea5ad3d9aad8b0ce45cc107429ba3e5","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        #"},{"line_number":311,"context_line":"        # Firstly we track the default for any \u003cdomain\u003e child without"},{"line_number":312,"context_line":"        # its own \u003cemulator\u003e sub-child:"},{"line_number":313,"context_line":"        self.emulator \u003d None"},{"line_number":314,"context_line":"        #"},{"line_number":315,"context_line":"        # Also per-\u003cdomain\u003e overrides for the default in self.emulator."},{"line_number":316,"context_line":"        # The dict maps domain types such as \u0027kvm\u0027 to the emulator"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_06481985","line":313,"updated":"2019-03-05 15:24:37.000000000","message":"consider calling this variable default_emulator to make it crystal clear that\u0027s what this is.","commit_id":"48d6753d37c0ef1ae5aa1ec9b5b369869ff8905b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4c8b97fb6ea5ad3d9aad8b0ce45cc107429ba3e5","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        # its own \u003cemulator\u003e sub-child:"},{"line_number":313,"context_line":"        self.emulator \u003d None"},{"line_number":314,"context_line":"        #"},{"line_number":315,"context_line":"        # Also per-\u003cdomain\u003e overrides for the default in self.emulator."},{"line_number":316,"context_line":"        # The dict maps domain types such as \u0027kvm\u0027 to the emulator"},{"line_number":317,"context_line":"        # path for that domain type.  Note that these overrides come"},{"line_number":318,"context_line":"        # from \u003cemulator\u003e elements under each \u003cdomain\u003e; there is no"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_c93542dd","line":315,"range":{"start_line":315,"start_character":19,"end_line":315,"end_character":27},"updated":"2019-03-05 15:24:37.000000000","message":"it\u0027s actually per domain *type*, no?","commit_id":"48d6753d37c0ef1ae5aa1ec9b5b369869ff8905b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4c8b97fb6ea5ad3d9aad8b0ce45cc107429ba3e5","unresolved":false,"context_lines":[{"line_number":317,"context_line":"        # path for that domain type.  Note that these overrides come"},{"line_number":318,"context_line":"        # from \u003cemulator\u003e elements under each \u003cdomain\u003e; there is no"},{"line_number":319,"context_line":"        # \u003cdomemulator\u003e element."},{"line_number":320,"context_line":"        self.domemulator \u003d dict()"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def parse_dom(self, xmldoc):"},{"line_number":323,"context_line":"        super(LibvirtConfigCapsGuest, self).parse_dom(xmldoc)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_e63715fe","line":320,"updated":"2019-03-05 15:24:37.000000000","message":"consider renaming this domain_type_emulator_map or domain_type_emulators to make it clearer what it contains/does.","commit_id":"48d6753d37c0ef1ae5aa1ec9b5b369869ff8905b"}]}