)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"3c2602bb484b5e323e2174b90f6c773858542719","unresolved":false,"context_lines":[{"line_number":5980,"context_line":"            info \u003d host.DomainJobInfo.for_domain(dom)"},{"line_number":5981,"context_line":""},{"line_number":5982,"context_line":"            if info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_NONE:"},{"line_number":5983,"context_line":"                self._lm_monitor_type_none(finish_event, guest, info, instance)"},{"line_number":5984,"context_line":""},{"line_number":5985,"context_line":"            if info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_NONE:"},{"line_number":5986,"context_line":"                # Migration is not yet started"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_511719a3","line":5983,"range":{"start_line":5983,"start_character":78,"end_line":5983,"end_character":79},"updated":"2016-06-06 16:00:32.000000000","message":"I don\u0027t really like this as you\u0027re obscuring what is actually happening here. In particular this method call is modifying the \u0027info.type\u0027 field, that is relied up later in this method, but that side-effect is now invisible to readers of this method.\n\nThis is a great example of the harm that refactoring code just to satisfy arbitrary complexity reports causes.\n\nAt the very least, the updating of \u0027info.type\u0027 should remain in this method, and not as a side effect in a called method.","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da92ff56bf5a93105a6c94599228026bca741791","unresolved":false,"context_lines":[{"line_number":5980,"context_line":"            info \u003d host.DomainJobInfo.for_domain(dom)"},{"line_number":5981,"context_line":""},{"line_number":5982,"context_line":"            if info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_NONE:"},{"line_number":5983,"context_line":"                self._lm_monitor_type_none(finish_event, guest, info, instance)"},{"line_number":5984,"context_line":""},{"line_number":5985,"context_line":"            if info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_NONE:"},{"line_number":5986,"context_line":"                # Migration is not yet started"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_c71cbd15","line":5983,"range":{"start_line":5983,"start_character":78,"end_line":5983,"end_character":79},"in_reply_to":"7aa08908_511719a3","updated":"2016-06-07 10:30:44.000000000","message":"Easily fixed","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"3c2602bb484b5e323e2174b90f6c773858542719","unresolved":false,"context_lines":[{"line_number":6009,"context_line":"                if (n % 10) \u003d\u003d 0:"},{"line_number":6010,"context_line":"                    self._lm_monitor_type_unbounded_log("},{"line_number":6011,"context_line":"                        info, instance, migration, n, now, progress_time,"},{"line_number":6012,"context_line":"                        progress_watermark)"},{"line_number":6013,"context_line":""},{"line_number":6014,"context_line":"                n +\u003d 1"},{"line_number":6015,"context_line":"            elif info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_COMPLETED:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_ec3aaacc","line":6012,"range":{"start_line":6012,"start_character":42,"end_line":6012,"end_character":43},"updated":"2016-06-06 16:00:32.000000000","message":"Again I think this is harming readability by obscuring side-effects. This method sounds like its just logging some data, but it is actually updating the migration object stats in the database too.","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da92ff56bf5a93105a6c94599228026bca741791","unresolved":false,"context_lines":[{"line_number":6009,"context_line":"                if (n % 10) \u003d\u003d 0:"},{"line_number":6010,"context_line":"                    self._lm_monitor_type_unbounded_log("},{"line_number":6011,"context_line":"                        info, instance, migration, n, now, progress_time,"},{"line_number":6012,"context_line":"                        progress_watermark)"},{"line_number":6013,"context_line":""},{"line_number":6014,"context_line":"                n +\u003d 1"},{"line_number":6015,"context_line":"            elif info.type \u003d\u003d libvirt.VIR_DOMAIN_JOB_COMPLETED:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_0717e5ed","line":6012,"range":{"start_line":6012,"start_character":42,"end_line":6012,"end_character":43},"in_reply_to":"7aa08908_ec3aaacc","updated":"2016-06-07 10:30:44.000000000","message":"Done","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"3c2602bb484b5e323e2174b90f6c773858542719","unresolved":false,"context_lines":[{"line_number":6187,"context_line":"               {\"remaining\": info.data_remaining,"},{"line_number":6188,"context_line":"                \"watermark\": progress_watermark,"},{"line_number":6189,"context_line":"                \"last\": (now - progress_time)}, instance\u003dinstance)"},{"line_number":6190,"context_line":""},{"line_number":6191,"context_line":"    def _clear_empty_migration(self, instance):"},{"line_number":6192,"context_line":"        try:"},{"line_number":6193,"context_line":"            del self.active_migrations[instance.uuid]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_ccac0ef0","line":6190,"updated":"2016-06-06 16:00:32.000000000","message":"Overall I don\u0027t really see this as a net win in maintainability at all. You\u0027ve got the exact same amount of code in the same structure, but you\u0027ve effectively just added a \"def some_method_name\" in the middle of it to satisfy an arbitrary code measure.","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da92ff56bf5a93105a6c94599228026bca741791","unresolved":false,"context_lines":[{"line_number":6187,"context_line":"               {\"remaining\": info.data_remaining,"},{"line_number":6188,"context_line":"                \"watermark\": progress_watermark,"},{"line_number":6189,"context_line":"                \"last\": (now - progress_time)}, instance\u003dinstance)"},{"line_number":6190,"context_line":""},{"line_number":6191,"context_line":"    def _clear_empty_migration(self, instance):"},{"line_number":6192,"context_line":"        try:"},{"line_number":6193,"context_line":"            del self.active_migrations[instance.uuid]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_c764bde6","line":6190,"in_reply_to":"7aa08908_ccac0ef0","updated":"2016-06-07 10:30:44.000000000","message":"Adding a \"def blah\" changes the structure. It defines a new and more limited scope, which can be understood as a unit independent from the rest of the _live_migration_monitor. Yes side-effects undermine this benefit and we should try to eliminate those but I didn\u0027t want to change too much with one patch and make it hard to review. I\u0027ve fixed the cases you mentioned, if you see additional issues I\u0027m happy to work through those.\n\nWe can also use follow-on patches to EG use an object instead of the hodgepodge of variables currently used for progress tracking.\n\nIf you truly find max-complexity meaningless, remove it from tox.ini, then I can abandon this patch.","commit_id":"60e862233aabc8a33d28153debe5e7663af6bc85"}]}