)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"bebda430d9fb1e7245a10eb4c2011f923abdeac9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"81f3c8c9_613d07eb","updated":"2023-08-22 09:26:01.000000000","message":"LGTM, but ideally Jesse agree with my comment and we can mark this as resolved.","commit_id":"6a0ea04e1071ce21cdb28ac916ca5847ed33d256"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"0a5ff8413a76007b9646464098f73818619afbd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"df9ff254_3b6e1a0b","updated":"2023-07-12 10:36:16.000000000","message":"recheck","commit_id":"6a0ea04e1071ce21cdb28ac916ca5847ed33d256"}],"tools/copy_role_params.py":[{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"f854d54a21bf18a2c9e0ac2c4c84fee273ed0bfe","unresolved":true,"context_lines":[{"line_number":150,"context_line":"    with open(new_role_env_file, \u0027w\u0027) as new_file:"},{"line_number":151,"context_line":"        # I don\u0027t think we can blindly dump here, if one is empty we skip dumping"},{"line_number":152,"context_line":"        dump_var \u003d {}"},{"line_number":153,"context_line":"        if (pd !\u003d {}):"},{"line_number":154,"context_line":"            dump_var[\u0027parameter_defaults\u0027] \u003d pd"},{"line_number":155,"context_line":"        if (rr !\u003d {}):"},{"line_number":156,"context_line":"            dump_var[\u0027resource_registry\u0027] \u003d rr"},{"line_number":157,"context_line":"        if (dump_var !\u003d {}):"}],"source_content_type":"text/x-python","patch_set":5,"id":"2963204f_d083496d","line":154,"range":{"start_line":153,"start_character":0,"end_line":154,"end_character":47},"updated":"2023-07-06 08:39:41.000000000","message":"Here we check if pd is empty, then later we\u0027re creating this key if it is empty.\n\nWhy not just switch this to:\n\n  dump_var[\u0027parameter_defaults\u0027] \u003d pd\n\nThen line 157 should never be empty, son that conditional also goes away.","commit_id":"8cc4bb1e8064533f12a10c88e738ec73f8465522"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"bebda430d9fb1e7245a10eb4c2011f923abdeac9","unresolved":true,"context_lines":[{"line_number":150,"context_line":"    with open(new_role_env_file, \u0027w\u0027) as new_file:"},{"line_number":151,"context_line":"        # I don\u0027t think we can blindly dump here, if one is empty we skip dumping"},{"line_number":152,"context_line":"        dump_var \u003d {}"},{"line_number":153,"context_line":"        if (pd !\u003d {}):"},{"line_number":154,"context_line":"            dump_var[\u0027parameter_defaults\u0027] \u003d pd"},{"line_number":155,"context_line":"        if (rr !\u003d {}):"},{"line_number":156,"context_line":"            dump_var[\u0027resource_registry\u0027] \u003d rr"},{"line_number":157,"context_line":"        if (dump_var !\u003d {}):"}],"source_content_type":"text/x-python","patch_set":5,"id":"810409f0_34721ab0","line":154,"range":{"start_line":153,"start_character":0,"end_line":154,"end_character":47},"in_reply_to":"2963204f_d083496d","updated":"2023-08-22 09:26:01.000000000","message":"hum .... let\u0027s see:\n\nif:\n\n   pd \u003c- empty (L154 doesn\u0027t happen and dump_var \u003c- empty)\n   rr \u003c- empty (L156 doesn\u0027t happen and dump_var \u003c- empty)\n\nthen:\n\n   dump_var \u003c- empty\n\nSo L157 is false and we jump to L162 and dump an empty file.\n\nSo I think that \"L157 should never be empty\" is not correct and that conditional is required.\n\nDo I miss something ?","commit_id":"8cc4bb1e8064533f12a10c88e738ec73f8465522"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"1a7bafb41d3db459afa6943830c75116cad8977a","unresolved":true,"context_lines":[{"line_number":150,"context_line":"    with open(new_role_env_file, \u0027w\u0027) as new_file:"},{"line_number":151,"context_line":"        # I don\u0027t think we can blindly dump here, if one is empty we skip dumping"},{"line_number":152,"context_line":"        dump_var \u003d {}"},{"line_number":153,"context_line":"        if (pd !\u003d {}):"},{"line_number":154,"context_line":"            dump_var[\u0027parameter_defaults\u0027] \u003d pd"},{"line_number":155,"context_line":"        if (rr !\u003d {}):"},{"line_number":156,"context_line":"            dump_var[\u0027resource_registry\u0027] \u003d rr"},{"line_number":157,"context_line":"        if (dump_var !\u003d {}):"}],"source_content_type":"text/x-python","patch_set":5,"id":"f23b5a47_50ed2d6a","line":154,"range":{"start_line":153,"start_character":0,"end_line":154,"end_character":47},"in_reply_to":"810409f0_34721ab0","updated":"2023-08-22 09:45:15.000000000","message":"What I was calling out is that we\u0027re doing something like this:\n\n  create empty thing\n  populate it with parameter content, if there are parameters\n  else create structured empty thing\n  return thing\n\nWhen I think we could do this instead:\n\n  create structured empty thing\n  populate it with parameter content, if there are parameters\n  return thing\n\nThat said, this is a matter of style and a small optimisation - hardly worth blocking given that this patch as-is has gone through significant testing.","commit_id":"8cc4bb1e8064533f12a10c88e738ec73f8465522"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"34f10e945b313e925c362649c54009190ede23e0","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    with open(new_role_env_file, \u0027w\u0027) as new_file:"},{"line_number":151,"context_line":"        # I don\u0027t think we can blindly dump here, if one is empty we skip dumping"},{"line_number":152,"context_line":"        dump_var \u003d {}"},{"line_number":153,"context_line":"        if (pd !\u003d {}):"},{"line_number":154,"context_line":"            dump_var[\u0027parameter_defaults\u0027] \u003d pd"},{"line_number":155,"context_line":"        if (rr !\u003d {}):"},{"line_number":156,"context_line":"            dump_var[\u0027resource_registry\u0027] \u003d rr"},{"line_number":157,"context_line":"        if (dump_var !\u003d {}):"}],"source_content_type":"text/x-python","patch_set":5,"id":"989c4f5e_d9d6dc07","line":154,"range":{"start_line":153,"start_character":0,"end_line":154,"end_character":47},"in_reply_to":"f23b5a47_50ed2d6a","updated":"2023-08-23 10:19:01.000000000","message":"ack.","commit_id":"8cc4bb1e8064533f12a10c88e738ec73f8465522"}]}