Closed thomasmfish closed 1 year ago
I've ~merged~ cherry-picked four of those but I don't think the other two are quite right. I'll leave a comment on the individual commits.
Great, thanks!
I just noticed this was still open - it can be closed, right?
What happened about the two which David thought were "not quite right"? But yes please do close the issue.
I'm not sure what happened here. I though I had left comments on the commits that I didn't cherry-pick but I can't find them anymore. So I'm adding them here:
try/catch
but the file will already be closed since this at the very end of the function. The block could be improved for sure but I don't think this change was a improvement in the right direction.The four commits that I picked were:
Closing as fixed.
I've found the comments now (they're on the commit but only if you access through the URLs https://github.com/MicronOxford/cockpit/commit/commit-hash
- if you try https://github.com/[thomasmfish](https://github.com/thomasmfish)/cockpit/commit/same-commit-hash
they're not there).
Anyway, there was a small detail this comment https://github.com/MicronOxford/cockpit/commit/1c5ec9c1c35439b90152bd20d5876d061d18f87a#commitcomment-89525659 about replacing file
with open
which I missed. I've now also merged it.
I've made a branch of some changes here that are small and could easily be cherry-picked in if that'd be helpful. Nothing that has caused me issues but a few things there could break error reporting.