-
Notifications
You must be signed in to change notification settings - Fork 20
Drop Ruby 3.2, support Ruby 4.0 #372
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
Conversation
0e52446 to
b7e3e4a
Compare
| bundle exec rb-sys-dock --platform ${{ matrix.rubyPlatform }} --ruby-versions "3.2,3.3,3.4" --mount-toolchains --build | ||
| gem install rb_sys --no-document | ||
| rb-sys-dock --platform ${{ matrix.rubyPlatform }} --ruby-versions "3.3,3.4,4.0" --mount-toolchains --build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to be changed from bundle exec because bundle exec did a bundle install which was leaking GH-runner-built gems into the rb-sys-dock container causing glibc mismatches
chris-olszewski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me.
Spot checked a few of the steep:ignore and they do all seem like false positives. Do you think it would worthwhile to tags the ignore with the specific errors?
| class ExternallyImmutableHash < SimpleDelegator | ||
| def initialize(initial_hash) | ||
| super(initial_hash.freeze) | ||
| super(initial_hash.freeze) # steep:ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea on why this became an ArgumentTypeMismatch with the steep bump?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. In RBS, we are calling this a Hash and there is not a typed initializer of hash that is valuable here. This is technically an initializer of SimpleDelegator, but RBS has no way of representing SimpleDelegator as what it truly is, so we lie. Newer Steep catches that this is an invalid super call for Hash even though we know it's valid for a delegator.
Maybe, at least for non-test code. Will look into doing so. |
What was changed
build-gemsworkflow to drop 3.2 and add 4.0ciworkflow to do 3.3-4.0 instead of 3.2-3.4rb-sysandmagnus, and then updated associated Rust code neededrbsandsteep, and then updated a lot of sig code and some runtime code to conform to newer rulesNOTE: Waiting until release to update README with the changes in supported versions. Once this is approved, will adjust the "required checks" to fit the newer check names.
Checklist