Conversation
| It means one of the file bumped to `typed: #{to}` made Sorbet crash. | ||
| Run `spoom bump -f` locally followed by `bundle exec srb tc` to investigate the problem. | ||
| ERR | ||
| undo_changes(files_to_bump, from) |
There was a problem hiding this comment.
I notice we have a lot of undo_changes in this method. I think a great refactor would be to move those to an ensure block so that they run whatever the exit code is.
Something like:
def foo
files_to_undo = []
change_some_files
files_to_undo = files_to_bump
# lot of complex code that could exit early
# if in dry mode, we want to undo all files
files_to_undo = files_with_errors unless dry
# some other output code
exit(files_changed.empty?)
ensure
undo_changes(files_to_undo, from)
endThere was a problem hiding this comment.
That would be cleaner indeed but Sorbet doesn't play nicely with ensure blocks and early return/exit: https://sorbet.run/#%23%20typed%3A%20true%0A%0Adef%20foo%0A%20%20dry%20%3D%20ARGV.first%0A%20%20exit%0Aensure%0A%20%20puts%20%22ensure%22%20if%20dry%0Aend%0A%0Adef%20bar%0A%20%20dry%20%3D%20ARGV.first%0A%20%20return%0Aensure%0A%20%20puts%20%22ensure%22%20if%20dry%0Aend%0A%0Adef%20baz%0A%20%20dry%20%3D%20ARGV.first%0Aensure%0A%20%20puts%20%22ensure%22%20if%20dry%0Aend. We would need to nest begin/ensure blocks and move the variable declaration before the begin which end up being messier 😬
There was a problem hiding this comment.
do we really need conditionals in the ensure block? this seems to work: https://sorbet.run/#%23%20typed%3A%20true%0A%0Adef%20boop%0A%20%20dry%20%3D%20ARGV.first%0A%20%20foo%20%3D%20%5B%5D%0A%0A%20%20return%20if%20dry%0A%0A%20%20foo%20%3D%20%5B%3Aa%2C%20%3Ab%5D%0Aensure%0A%20%20foop%28foo%29%0Aend%0A%0Adef%20foop%28x%29%0Aend
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Ensure sorbet doesn't segfault when we run it with
spoom tcspoom bumpAnd display a meaningful message to the user to avoid confusion.