Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require actual version of sorbet-static-and-runtime as minimum #1260

Merged
merged 5 commits into from
Nov 7, 2022

Conversation

sambostock
Copy link
Contributor

@sambostock sambostock commented Nov 4, 2022

Motivation

While sorbet-static and sorbet-runtime versions 0.5.9204 do exist, 0.5.9519 is the first version of sorbet-static-and-runtime published on RubyGems, so we cannot depend on anything lesser than that.

We can potentially require an even newer version, as discussed in #1252 (comment), but I'm not sure which, so I've not made that change yet.
As per @paracycle's comment, I've gone ahead and further bumped the minimum version requirement to a version that allows us to get rid of our feature checks.

Implementation

This updates the version in tapioca.gemspec and refreshes Gemfile.lock accordingly.

Tests

All existing tests apply.

While `sorbet-static` and `sorbet-runtime` versions `0.5.9204` do exist,
`0.5.9519` is the first version of `sorbet-static-and-runtime` published
on RubyGems, so we cannot depend on anything lesser than that.
@sambostock sambostock added the dependencies Pull requests that update a dependency file label Nov 4, 2022
@sambostock sambostock requested a review from Morriar November 4, 2022 20:03
@sambostock sambostock requested a review from a team as a code owner November 4, 2022 20:03
@paracycle
Copy link
Member

If we can bump the minimum version to >= 0.5.9892 (which seems to date back to April 2022) then we can get rid of all of these feature detection workarounds as well: https://github.com/Shopify/tapioca/blob/main/lib/tapioca/helpers/sorbet_helper.rb#L23-L25

This feature is supported from Sorbet versions 0.5.9220 onwards, and
since we require at least 0.5.9519, we know it is supported.
@sambostock
Copy link
Contributor Author

@paracycle, I've added commits getting rid of a feature flag by successively bumping the version, and added a documentation comment to the now empty hash (as I imagine we want to keep it around for future use).

Alternatively, I could delete all the feature checking code, if that's not something we want to keep around.

By bumping the minimum Sorbet version, we can avoid the feature check.
By bumping the minimum Sorbet version, we can avoid the feature check.
Rather than remove the feature check code, we document it for future
use.
@sambostock sambostock force-pushed the bump-sorbet-static-and-runtime-requirement branch from 029a352 to f0aadd0 Compare November 5, 2022 01:26
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Have we verified the new generics syntax on Core?

@sambostock
Copy link
Contributor Author

I don't know about the generics syntax specifically, but trying out this branch on Core passes RBI related CI checks. It's already on Sorbet 0.5.10482.

@sambostock sambostock merged commit e2fdfe3 into main Nov 7, 2022
@sambostock sambostock deleted the bump-sorbet-static-and-runtime-requirement branch November 7, 2022 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants