Skip to content

Use new no error sections option#77

Merged
vinistock merged 1 commit intomasterfrom
use_new_no_errors_option
Mar 18, 2021
Merged

Use new no error sections option#77
vinistock merged 1 commit intomasterfrom
use_new_no_errors_option

Conversation

@vinistock
Copy link
Member

Use the new option introduced in sorbet/sorbet#4078 for Spoom bump.

I locked the version of Sorbet to be greater or equal to the one where the new option is introduced.

@vinistock vinistock force-pushed the use_new_no_errors_option branch from c6a8019 to 51422cb Compare March 16, 2021 21:21
o.close
err = e.read.to_s
e.close
i.close
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was breaking locally with an error related to having too many files open from Open3.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch ❤️

Error: Sorbet returned typechecking errors for `/errors.rb`
8:0-8:11: Not enough arguments provided for method `Foo#foo`. Expected: `1`, got: `0` (7004)
4:2-5:5: Returning value that does not conform to method result type (7005)
4:2-5:5: Expected `String` but found `NilClass` for method result type (7005)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changed after the Sorbet upgrade.

o.close
err = e.read.to_s
e.close
i.close
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch ❤️

@Morriar
Copy link
Contributor

Morriar commented Mar 16, 2021

Looks like the Sorbet bump also created errors around blocks: https://github.com/Shopify/spoom/pull/77/checks?check_run_id=2125606434 we might have to update those definitions upstream :/

Copy link
Contributor

@RyanBrushett RyanBrushett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your CI failures are related to upstream RBI files defining a method that is missing an explicit block param. Happy to 🍐 to fix those if you're interested!

@vinistock
Copy link
Member Author

Fixed the types for with_clean_env and some other related methods in sorbet/sorbet#4092. Now waiting for the release to update this PR.

@vinistock vinistock force-pushed the use_new_no_errors_option branch from 51422cb to 50dfb89 Compare March 18, 2021 13:25
@vinistock vinistock merged commit d2b4e9c into master Mar 18, 2021
@vinistock vinistock deleted the use_new_no_errors_option branch March 18, 2021 13:36
@vinistock vinistock temporarily deployed to production March 19, 2021 18:53 Inactive
@Morriar Morriar added the enhancement New feature or request label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants