)]}'
{"nova/network/neutronv2/api.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ae5294257613e9b94b2acecc561df1564ceba4e","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                LOG.exception(_LE(\u0027Neutron error creating port on network %s\u0027),"},{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _populate_mac_address(self, instance, port_req_body, available_macs):"},{"line_number":247,"context_line":"        if available_macs is not None:"},{"line_number":248,"context_line":"            if not available_macs:"},{"line_number":249,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_800b5fd5","line":246,"updated":"2016-06-08 13:44:22.000000000","message":"We could probably make it as staticmethod. Then It should be nice to add a doctext with arguments, returns values and possible exception raised","commit_id":"a172fe8a56224c9e32a9ecd941ec0c7aa6beb8a1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9b7c59a30cfbbc7f4318af8da52c4f196c428eb3","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                LOG.exception(_LE(\u0027Neutron error creating port on network %s\u0027),"},{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _populate_mac_address(self, instance, port_req_body, available_macs):"},{"line_number":247,"context_line":"        if available_macs is not None:"},{"line_number":248,"context_line":"            if not available_macs:"},{"line_number":249,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_9538f8a1","line":246,"in_reply_to":"7aa08908_117c5571","updated":"2016-06-13 10:31:03.000000000","message":"We can add them, its just we are bad at maintaining them.\n\nIdeally the signature tells you all you need to know about the method.\n\nPublic methods, its often better to document those.\n\nBut thats just my personal take.","commit_id":"a172fe8a56224c9e32a9ecd941ec0c7aa6beb8a1"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"822ba7ac582a97601933ddd06273f4888abdd2fa","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                LOG.exception(_LE(\u0027Neutron error creating port on network %s\u0027),"},{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _populate_mac_address(self, instance, port_req_body, available_macs):"},{"line_number":247,"context_line":"        if available_macs is not None:"},{"line_number":248,"context_line":"            if not available_macs:"},{"line_number":249,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_117c5571","line":246,"in_reply_to":"7aa08908_5676613e","updated":"2016-06-10 16:42:51.000000000","message":"I thought we could add docs for non-static method either, what is root cause of outdated comments?","commit_id":"a172fe8a56224c9e32a9ecd941ec0c7aa6beb8a1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"4d3ad3faf31b1486117dce5d884ec0838f5fe12d","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                LOG.exception(_LE(\u0027Neutron error creating port on network %s\u0027),"},{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _populate_mac_address(self, instance, port_req_body, available_macs):"},{"line_number":247,"context_line":"        if available_macs is not None:"},{"line_number":248,"context_line":"            if not available_macs:"},{"line_number":249,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_5676613e","line":246,"in_reply_to":"7aa08908_800b5fd5","updated":"2016-06-08 14:07:13.000000000","message":"yeah, we can make it a staticmethod.\n\nI just hate those docs, as they drift out of sync.","commit_id":"a172fe8a56224c9e32a9ecd941ec0c7aa6beb8a1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0be25380761d566afe78e1d8bba8e8a698ff4c70","unresolved":false,"context_lines":[{"line_number":200,"context_line":"                if not available_macs:"},{"line_number":201,"context_line":"                    raise exception.PortNotFree("},{"line_number":202,"context_line":"                        instance\u003dinstance.uuid)"},{"line_number":203,"context_line":"                mac_address \u003d available_macs.pop()"},{"line_number":204,"context_line":"                port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac_address"},{"line_number":205,"context_line":"            if dhcp_opts is not None:"},{"line_number":206,"context_line":"                port_req_body[\u0027port\u0027][\u0027extra_dhcp_opts\u0027] \u003d dhcp_opts"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_b8d8fd80","side":"PARENT","line":203,"range":{"start_line":203,"start_character":16,"end_line":203,"end_character":27},"updated":"2016-06-15 17:19:49.000000000","message":"In your new version, mac_address will be set in all cases, I like that.","commit_id":"63ed9657a1e6a5612c80e821dac01621732b2bde"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"55c33f3187c21d9a6248b1fbdbc66586849a37ba","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_2d0aed21","line":247,"updated":"2016-06-22 15:14:32.000000000","message":"Well, we already have a same method name for a different signature :\nhttps://github.com/openstack/nova/blob/9e176a66732beda5099dc15787bf8fe1acd5b865/nova/network/neutronv2/api.py#L698\n\ncould you please rename that one to prevent any issue ?","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"73b4e4da8e4c6c1d29b396f0b36094be9502c42a","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_18ce4db4","line":247,"in_reply_to":"3aaa91ec_0df731f6","updated":"2016-06-22 15:35:15.000000000","message":"I like to avoid comments if I can, as then tend to drift out of date.\n\nI didn\u0027t add one here as all the \"_populate\" methods tend to modify port_req_body, like _populate_neutron_extension_values.\n\nThis will soon move next to all those other populate calls, so it looks a bit more obvious what is happening once its moved.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bb38dcf0a4c2c2c290eb7968f00585b1f42d877c","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_2d6f8dc6","line":247,"in_reply_to":"3aaa91ec_2d0aed21","updated":"2016-06-22 15:15:35.000000000","message":"Also, I think we should explain that this method is also modifying an mutable argument and not just returning a field.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9078a2067649375ab5b8366671034b63a2a8a367","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                              network_id, instance\u003dinstance)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_0df731f6","line":247,"in_reply_to":"3aaa91ec_2d6f8dc6","updated":"2016-06-22 15:17:41.000000000","message":"Oh oops, I forgot you modified the other method name in the previous patch. That said, I think we should explain that this method would update port_req_body.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0be25380761d566afe78e1d8bba8e8a698ff4c70","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("},{"line_number":251,"context_line":"                    instance\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_b863dddc","line":248,"range":{"start_line":248,"start_character":8,"end_line":248,"end_character":38},"updated":"2016-06-15 17:19:49.000000000","message":"nit:  Consider this:\n\n  if available_macs is None:\n      return\n  if not available_macs:\n      raise ...\n  ...\n\nBut, it really isn\u0027t a big deal and might just be my own style thing.  I just like it because it reduces indentation and communicates \"Hey, in this case, we have nothing else to do.  Just return.\"","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"73b4e4da8e4c6c1d29b396f0b36094be9502c42a","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("},{"line_number":251,"context_line":"                    instance\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_f8ba61e0","line":248,"range":{"start_line":248,"start_character":8,"end_line":248,"end_character":38},"in_reply_to":"3aaa91ec_3d07c21d","updated":"2016-06-22 15:35:15.000000000","message":"yeah, I like that better too, but yeah, its trying to be a minimal change move at this point.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51f7d488526b68aa131f1c560bb0be852588526f","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("},{"line_number":251,"context_line":"                    instance\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3aaa91ec_3d07c21d","line":248,"range":{"start_line":248,"start_character":8,"end_line":248,"end_character":38},"in_reply_to":"7aa08908_595d9878","updated":"2016-06-22 13:44:21.000000000","message":"What Ed said.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c62f7d6d8cff56c133915b3ff8d7d5e2518433f3","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _populate_mac_address(instance, port_req_body, available_macs):"},{"line_number":248,"context_line":"        if available_macs is not None:"},{"line_number":249,"context_line":"            if not available_macs:"},{"line_number":250,"context_line":"                raise exception.PortNotFree("},{"line_number":251,"context_line":"                    instance\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_595d9878","line":248,"range":{"start_line":248,"start_character":8,"end_line":248,"end_character":38},"in_reply_to":"7aa08908_b863dddc","updated":"2016-06-15 19:37:34.000000000","message":"@Carl - that is a better (IMO) design, but since this is just about moving this method, it\u0027s cleaner if the code isn\u0027t changing.","commit_id":"d4e2752a67edabbefde45e1c82cb6a4fa2d2d012"}]}