)]}'
{"mistral/actions/openstack/action_generator/base.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"242114374572546dfc28b5bb94fc96a95f01684c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        if \u0027_comment\u0027 in map_part:"},{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    with open(pkg.resource_filename("},{"line_number":44,"context_line":"              version.version_info.package, MAPPING_PATH)) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    for k, v in mapping.items():"},{"line_number":48,"context_line":"        if isinstance(v, dict):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_b564b3e5","line":45,"range":{"start_line":43,"start_character":4,"end_line":45,"end_character":31},"updated":"2016-12-08 06:58:39.000000000","message":"Honestly, I don\u0027t understand the critical importance of this fix. Is it just to get rid of read() method?\n\nThe previous version also looks correct. And it also takes 3 lines of code.","commit_id":"1c871d86305b0f617d1c7ddcd2c4a08048eb85d4"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"4f53c77dbb164a4e32177648d85b4ebf450153e9","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        if \u0027_comment\u0027 in map_part:"},{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    with open(pkg.resource_filename("},{"line_number":44,"context_line":"              version.version_info.package, MAPPING_PATH)) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    for k, v in mapping.items():"},{"line_number":48,"context_line":"        if isinstance(v, dict):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_783f72d6","line":45,"range":{"start_line":43,"start_character":4,"end_line":45,"end_character":31},"in_reply_to":"3a71b18c_75d5bbb7","updated":"2016-12-08 07:54:44.000000000","message":"OOh, ok, absolutely agree on open/close thought. Then I think the message commit is misleading because this concern with proper file closure is much more important for me than whether use json.loads or json.load. Agree?\n\nPlease make it more precise.","commit_id":"1c871d86305b0f617d1c7ddcd2c4a08048eb85d4"},{"author":{"_account_id":22514,"name":"fengchaoyang","email":"fengchaoyang@unitedstack.com","username":"fengchaoyang"},"change_message_id":"7a667d77327f3e3065c49b9d283ab93576754451","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        if \u0027_comment\u0027 in map_part:"},{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    with open(pkg.resource_filename("},{"line_number":44,"context_line":"              version.version_info.package, MAPPING_PATH)) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    for k, v in mapping.items():"},{"line_number":48,"context_line":"        if isinstance(v, dict):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_75d5bbb7","line":45,"range":{"start_line":43,"start_character":4,"end_line":45,"end_character":31},"in_reply_to":"3a71b18c_b564b3e5","updated":"2016-12-08 07:21:50.000000000","message":"It is good practice to use the with keyword when dealing with file objects. This has the advantage that the file is properly closed after its suite finishes, even if an exception is raised on the way.(from https://docs.python.org/2/tutorial/inputoutput.html#)\nThe previous version looks correct, it opens a file,but not call f.close() to close the file.","commit_id":"1c871d86305b0f617d1c7ddcd2c4a08048eb85d4"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"8ae0784deef06be1bc1f7c7a53f3b783b242ea81","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    with open(pkg.resource_filename("},{"line_number":44,"context_line":"              version.version_info.package, MAPPING_PATH)) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    for k, v in mapping.items():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_e46bdb83","line":44,"range":{"start_line":44,"start_character":14,"end_line":44,"end_character":42},"updated":"2016-12-08 09:31:45.000000000","message":"minor thing - IMO the \"with open\" code block will look better if this was a variable.\nI agree with this approach for all languages: http://stackoverflow.com/questions/568751/is-it-good-practice-to-create-once-used-variables \n\nThe total will still be 3 lines, but they can look like so:\n    package \u003d version.version_info.package\n    with open(pkg.resource_filename(package, MAPPING_PATH)) as fh:\n        mapping \u003d json.load(fh)","commit_id":"22464eac852e400ea2dbe9f10f6ac4ed9ae3dada"},{"author":{"_account_id":22514,"name":"fengchaoyang","email":"fengchaoyang@unitedstack.com","username":"fengchaoyang"},"change_message_id":"3b91174dc71ba3aeec91a13f98a958df3b662760","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    with open(pkg.resource_filename("},{"line_number":44,"context_line":"              version.version_info.package, MAPPING_PATH)) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    for k, v in mapping.items():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_0d41db39","line":44,"range":{"start_line":44,"start_character":14,"end_line":44,"end_character":42},"in_reply_to":"3a71b18c_e46bdb83","updated":"2016-12-08 09:43:06.000000000","message":"Yes, I agree with you:)","commit_id":"22464eac852e400ea2dbe9f10f6ac4ed9ae3dada"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"9354b375ec0eec51e5453a226ff826ec49d9c757","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        if \u0027_comment\u0027 in map_part:"},{"line_number":41,"context_line":"            del map_part[\u0027_comment\u0027]"},{"line_number":42,"context_line":"    mapping_file \u003d pkg.resource_filename("},{"line_number":43,"context_line":"        version.version_info.package, MAPPING_PATH)"},{"line_number":44,"context_line":"    with open(mapping_file) as fh:"},{"line_number":45,"context_line":"        mapping \u003d json.load(fh)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_315509c6","line":43,"range":{"start_line":43,"start_character":50,"end_line":43,"end_character":51},"updated":"2016-12-08 12:11:14.000000000","message":"if you want to go in this direction, then mistral code conventions require the closing bracts to be in a new line.\n\nHowever, I still vote for the 3 lines solution approach I suggested before:\n\n    package \u003d version.version_info.package\n    with open(pkg.resource_filename(package, MAPPING_PATH)) as fh:\n        mapping \u003d json.load(fh)","commit_id":"4df47d1af736da38cf05b23f1c4db13bb21d79e9"}]}