)]}'
{"metropolis/test/installer/main.go":[{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"1b647f208c040544dd32a570432e6b0833533edf","unresolved":true,"context_lines":[{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\tlog.Fatalf(\"Couldn\u0027t stat the installer EFI executable: %s\", err.Error())"},{"line_number":144,"context_line":"\t}"},{"line_number":145,"context_line":"\tbundle, err :\u003d os.Open(TestOSBundle)"},{"line_number":146,"context_line":"\tif err !\u003d nil {"},{"line_number":147,"context_line":"\t\tlog.Fatalf(\"failed to open TestOS bundle: %v\", err)"},{"line_number":148,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":10,"id":"4c23e90c_0ee3b0c1","line":145,"updated":"2021-11-11 16:25:26.000000000","message":"This could use a separate comment.","commit_id":"77886ff670511115a3aee9683f7ec472ffe0c99e"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"1ae80652077cddae1df170e9e302da42eacc1118","unresolved":false,"context_lines":[{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\tlog.Fatalf(\"Couldn\u0027t stat the installer EFI executable: %s\", err.Error())"},{"line_number":144,"context_line":"\t}"},{"line_number":145,"context_line":"\tbundle, err :\u003d os.Open(TestOSBundle)"},{"line_number":146,"context_line":"\tif err !\u003d nil {"},{"line_number":147,"context_line":"\t\tlog.Fatalf(\"failed to open TestOS bundle: %v\", err)"},{"line_number":148,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":10,"id":"4eb3b2ec_9b956bf4","line":145,"in_reply_to":"1265c307_83d587d6","updated":"2021-11-22 12:50:48.000000000","message":"Fair enough. I can work with that easily, though I can\u0027t tell anymore how much of it is due to my current familiarity with the project.\n\nRegarding that comment, I agree. I\u0027ll try to avoid this kind of references.","commit_id":"77886ff670511115a3aee9683f7ec472ffe0c99e"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"90f17bc4dc7f027bf9e04c819dfadc451540a3e4","unresolved":true,"context_lines":[{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\tlog.Fatalf(\"Couldn\u0027t stat the installer EFI executable: %s\", err.Error())"},{"line_number":144,"context_line":"\t}"},{"line_number":145,"context_line":"\tbundle, err :\u003d os.Open(TestOSBundle)"},{"line_number":146,"context_line":"\tif err !\u003d nil {"},{"line_number":147,"context_line":"\t\tlog.Fatalf(\"failed to open TestOS bundle: %v\", err)"},{"line_number":148,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":10,"id":"1265c307_83d587d6","line":145,"in_reply_to":"3854b7fe_1a83a73f","updated":"2021-11-18 16:36:35.000000000","message":"Sure, complicated is relative. But if opening a file and getting its file size to pass both of those into a documented struct passes that bar, the entire existing codebase is massively underdocumented. The origin of the file is clearly documented in Bazel and what it does is documented in package-level documentation.\n\nIf the intention is sufficiently more complex and/or involves things not visible from the code, that is a good reason to leave a comment explaining that.\nBut this code does exactly what it says it does and the intention was to open a file which is exactly what this does.\n\nThe comment above is IMO not that great because it calls out that it mimics metroctl, which is correct now but probably not for long. And nobody will update that comment because it\u0027s not in metroctl code and this test doesn\u0027t exercise metroctl itself, but the install flow.","commit_id":"77886ff670511115a3aee9683f7ec472ffe0c99e"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"ba6ea5475ba269fedf14bd27de38767d6a49909d","unresolved":true,"context_lines":[{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\tlog.Fatalf(\"Couldn\u0027t stat the installer EFI executable: %s\", err.Error())"},{"line_number":144,"context_line":"\t}"},{"line_number":145,"context_line":"\tbundle, err :\u003d os.Open(TestOSBundle)"},{"line_number":146,"context_line":"\tif err !\u003d nil {"},{"line_number":147,"context_line":"\t\tlog.Fatalf(\"failed to open TestOS bundle: %v\", err)"},{"line_number":148,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":10,"id":"62b53daa_4977b635","line":145,"in_reply_to":"4c23e90c_0ee3b0c1","updated":"2021-11-17 16:59:39.000000000","message":"This is IMO all pretty trivial code which shouldn\u0027t need comments.\n\nI guess we should come to some consensus on what sort of comment style we want in the codebase. I generally don\u0027t believe that commenting what the code does is useful unless it\u0027s very complicated. I generally comment a high-level overview of what a function does and annotate references if code is based on specs. I left an agenda item about this in next week\u0027s meeting.","commit_id":"77886ff670511115a3aee9683f7ec472ffe0c99e"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"509d66626b86e01c8f7eac7020593f8387fbff92","unresolved":true,"context_lines":[{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\tlog.Fatalf(\"Couldn\u0027t stat the installer EFI executable: %s\", err.Error())"},{"line_number":144,"context_line":"\t}"},{"line_number":145,"context_line":"\tbundle, err :\u003d os.Open(TestOSBundle)"},{"line_number":146,"context_line":"\tif err !\u003d nil {"},{"line_number":147,"context_line":"\t\tlog.Fatalf(\"failed to open TestOS bundle: %v\", err)"},{"line_number":148,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":10,"id":"3854b7fe_1a83a73f","line":145,"in_reply_to":"62b53daa_4977b635","updated":"2021-11-18 12:13:19.000000000","message":"\"Complicated\" is relative. I\u0027d rather make it easier for someone who\u0027s reading this code for the first time, maybe not even knowing that the specification exists at all. That\u0027s relevant, since I believe that quality open source code is one of this project\u0027s selling points.\n\nSecondly, it\u0027s not always about explaining what the code does, and more rather about stating the author\u0027s intention so that the implementation as a whole is easier to follow.","commit_id":"77886ff670511115a3aee9683f7ec472ffe0c99e"}]}