)]}'
{"pkg/phase/executors/k8s_applier.go":[{"author":{"_account_id":21991,"name":"Kostyantyn Kalynovskyi","email":"kkalinovskiy@gmail.com","username":"kkalynovskyi"},"change_message_id":"630e8f078d9d25e828f25508cf561db96119cda9","unresolved":true,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"// Run executor, should be performed in separate go routine"},{"line_number":77,"context_line":"func (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) {"},{"line_number":78,"context_line":"\tlog.Debug(\"Filtering out documents that shouldn\u0027t be applied to kubernetes from document bundle\")"},{"line_number":79,"context_line":"\tfilteredBundle, err :\u003d e.ExecutorBundle.SelectBundle(document.NewDeployToK8sSelector())"},{"line_number":80,"context_line":"\tif err !\u003d nil {"},{"line_number":81,"context_line":"\t\thandleError(ch, err)"},{"line_number":82,"context_line":"\t\tclose(ch)"},{"line_number":83,"context_line":"\t\treturn"},{"line_number":84,"context_line":"\t}"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"\tapplier, err :\u003d e.prepareApplier(ch)"},{"line_number":87,"context_line":"\tif err !\u003d nil {"},{"line_number":88,"context_line":"\t\thandleError(ch, err)"},{"line_number":89,"context_line":"\t\tclose(ch)"},{"line_number":90,"context_line":"\t\treturn"},{"line_number":91,"context_line":"\t}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"\tif e.cleanup !\u003d nil {"},{"line_number":94,"context_line":"\t\tdefer e.cleanup()"}],"source_content_type":"text/x-go","patch_set":10,"id":"97584adb_e6413319","line":91,"range":{"start_line":78,"start_character":0,"end_line":91,"end_character":2},"updated":"2021-05-06 19:22:03.000000000","message":"Could you please explain reasons for splitting the function into two? I thought this was easier to read with one handleError() call","commit_id":"177d16152a954b2060455e93cba9dc8a72420a90"},{"author":{"_account_id":3009,"name":"Vladimir Kozhukalov","email":"kozhukalov@gmail.com","username":"kozhukalov"},"change_message_id":"6c46485b44984a82cf4a434fae31f233f61a2ab0","unresolved":true,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"// Run executor, should be performed in separate go routine"},{"line_number":77,"context_line":"func (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) {"},{"line_number":78,"context_line":"\tlog.Debug(\"Filtering out documents that shouldn\u0027t be applied to kubernetes from document bundle\")"},{"line_number":79,"context_line":"\tfilteredBundle, err :\u003d e.ExecutorBundle.SelectBundle(document.NewDeployToK8sSelector())"},{"line_number":80,"context_line":"\tif err !\u003d nil {"},{"line_number":81,"context_line":"\t\thandleError(ch, err)"},{"line_number":82,"context_line":"\t\tclose(ch)"},{"line_number":83,"context_line":"\t\treturn"},{"line_number":84,"context_line":"\t}"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"\tapplier, err :\u003d e.prepareApplier(ch)"},{"line_number":87,"context_line":"\tif err !\u003d nil {"},{"line_number":88,"context_line":"\t\thandleError(ch, err)"},{"line_number":89,"context_line":"\t\tclose(ch)"},{"line_number":90,"context_line":"\t\treturn"},{"line_number":91,"context_line":"\t}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"\tif e.cleanup !\u003d nil {"},{"line_number":94,"context_line":"\t\tdefer e.cleanup()"}],"source_content_type":"text/x-go","patch_set":10,"id":"0aa086cf_2a247740","line":91,"range":{"start_line":78,"start_character":0,"end_line":91,"end_character":2},"in_reply_to":"97584adb_e6413319","updated":"2021-05-11 09:15:43.000000000","message":"The reason for this is that reading bundle is not related to the preparing applier. The prepareApplier method just returns applier and bundle and they are not related to each other. This is of course subjective. If you insist it (reading bundle) to be a part prepareApplier method, I can revert this change. Or we can do even more, prepareApplier can return a function instead of applier and then inside the Run method we can just call this function with the applier options\n```\nfunc (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) {\n...\n\tapplyFunc, err :\u003d e.prepareApplier(ch)\n...\n\tapplyFunc(applyOptions)\n}\n\nfunc (e *KubeApplierExecutor) prepareApplier(ch chan events.Event) (func(k8sapplier.ApplyOptions), error) {\n...\n\tfilteredBundle, err :\u003d e.ExecutorBundle.SelectBundle(document.NewDeployToK8sSelector())\n...\n\treturn func(ao k8sapplier.ApplyOptions) {\n\t\tk8sapplier.NewApplier(ch, factory).ApplyBundle(filteredBundle, ao)\n\t}, nil\n}\n```","commit_id":"177d16152a954b2060455e93cba9dc8a72420a90"},{"author":{"_account_id":21991,"name":"Kostyantyn Kalynovskyi","email":"kkalinovskiy@gmail.com","username":"kkalynovskyi"},"change_message_id":"630e8f078d9d25e828f25508cf561db96119cda9","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\t\treturn"},{"line_number":91,"context_line":"\t}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"\tif e.cleanup !\u003d nil {"},{"line_number":94,"context_line":"\t\tdefer e.cleanup()"},{"line_number":95,"context_line":"\t}"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"\tdryRunStrategy :\u003d common.DryRunNone"},{"line_number":98,"context_line":"\tif runOpts.DryRun {"}],"source_content_type":"text/x-go","patch_set":10,"id":"a6b236f2_90a2629c","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":2},"updated":"2021-05-06 19:22:03.000000000","message":"lets make it safe to use without any if statements:\nadd default cleanup when u create the executor object. Alternatively, prepare can return cleanup function instead of making it part of the executor struct. Initially i added it as part of the struct, because prepare function had too long return signature, if you make it shorter, you can as well return cleanup function","commit_id":"177d16152a954b2060455e93cba9dc8a72420a90"},{"author":{"_account_id":3009,"name":"Vladimir Kozhukalov","email":"kozhukalov@gmail.com","username":"kozhukalov"},"change_message_id":"6c46485b44984a82cf4a434fae31f233f61a2ab0","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\t\treturn"},{"line_number":91,"context_line":"\t}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"\tif e.cleanup !\u003d nil {"},{"line_number":94,"context_line":"\t\tdefer e.cleanup()"},{"line_number":95,"context_line":"\t}"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"\tdryRunStrategy :\u003d common.DryRunNone"},{"line_number":98,"context_line":"\tif runOpts.DryRun {"}],"source_content_type":"text/x-go","patch_set":10,"id":"9b3a3cbc_304c9e86","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":2},"in_reply_to":"a6b236f2_90a2629c","updated":"2021-05-11 09:15:43.000000000","message":"Yes, sure, you are right, it is better to have a default cleanup instead of using if clause.","commit_id":"177d16152a954b2060455e93cba9dc8a72420a90"},{"author":{"_account_id":3009,"name":"Vladimir Kozhukalov","email":"kozhukalov@gmail.com","username":"kozhukalov"},"change_message_id":"ea7779cde7a56eb73dae309cfe116fa0d36fb403","unresolved":true,"context_lines":[{"line_number":79,"context_line":"// Run executor, should be performed in separate go routine"},{"line_number":80,"context_line":"func (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) {"},{"line_number":81,"context_line":"\tdefer close(ch)"},{"line_number":82,"context_line":"\tdefer e.cleanup()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"\tapplier, filteredBundle, err :\u003d e.prepareApplier(ch)"},{"line_number":85,"context_line":"\tif err !\u003d nil {"}],"source_content_type":"text/x-go","patch_set":12,"id":"f8c1fca6_83587f6d","line":82,"updated":"2021-05-13 08:13:36.000000000","message":"My fault here. The pointer to the method is evaluated here and the default cleanup will be called despite the fact it is later updated.","commit_id":"13ac452cf369690adf2a28064165d826fd77f60d"}]}