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

Support Range#step behavior change in Ruby 3.4 #1958

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

zverok
Copy link
Contributor

@zverok zverok commented Aug 10, 2024

This is a PR to support this change in Ruby: ruby/ruby#7444 (the design is approved by Matz, and the code itself is approved by the Ruby core team).

I am not sure how this actually should be addressed, so I will be thankful for any advice.

Basically, the essence of the change is that:

  • Before Ruby 3.4, Ruby#step’s type signature was step(Int step?)
  • After the linked PR, the signature would become step(Addable step), with Addable being such that Range#begin + step will work and produce the next value.

So, my questions are:

  1. How to describe that in RBS syntax?
  2. How to adjust RBS core/range.rbs to specify that the signature was changed between 3.3 and 3.4?
  3. How to proceed with those two PRs (into Ruby itself and into rbs)?

Please advise 🙏

@zverok zverok self-assigned this Aug 10, 2024
@ParadoxV5
Copy link
Contributor

ParadoxV5 commented Aug 10, 2024

  1. How to describe that in RBS syntax?

It would be using an interface that’s along the lines of

interface _Addable[T]
  def +: (T other) -> T
end

@sampersand has ongoing interest in interfaces [P.S. and Ruby core RBSs in general] like this; it’d be conveient to coördinate this addition with him.

  1. How to adjust RBS core/range.rbs to specify that the signature was changed between 3.3 and 3.4?

RBS files doesn’t have Ruby versioning currently and the latest versions only track the latest Ruby release.
However, release notes include statements on updating Ruby version and links to PRs.

  1. How to proceed with those two PRs (into Ruby itself and into rbs)?

Soutaro released RBS 3.4 nearing Ruby 3.3’s release as the new bundled version.

I say, proceed with the Ruby side first.
RBSs in this repo (or at least those for the Ruby core) also include a copy of Ruby’s docs; so if the Ruby doc mentions using #+ during Ruby 3.4’s preview, it gives Soutaro time to update the docs here for an accompanying RBS release.

@zverok
Copy link
Contributor Author

zverok commented Aug 11, 2024

I say, proceed with the Ruby side first.

I see. My concern here is that due to this “incompatibility”, the core Ruby CI is red (the test-bundled-gems task), therefore if I’ll merge it into the main branch, then it would be red for everybody till the rbs would be adjusted.

So I am looking for a way to mitigate it.

@soutaro
Copy link
Member

soutaro commented Aug 13, 2024

@zverok Thank you for letting us know the change.

How about using rbs_skip_tests in Ruby repo to put your change there? You can skip tests of RBS gem in Ruby repo, and then we can update the RBS tests.

@zverok
Copy link
Contributor Author

zverok commented Aug 18, 2024

@soutaro Thank you for your help! 🙏

I used rbs_skip_tests solution, and now the PR is merged 🎉

@soutaro soutaro mentioned this pull request Aug 22, 2024
@soutaro soutaro added this to the RBS 3.8 milestone Dec 18, 2024
@soutaro
Copy link
Member

soutaro commented Dec 19, 2024

I'm taking over this PR.

@soutaro soutaro force-pushed the range-step-behavior-change branch from 775a90e to bc6b2b0 Compare December 19, 2024 02:41
@soutaro soutaro marked this pull request as ready for review December 19, 2024 02:41
@soutaro soutaro enabled auto-merge December 19, 2024 02:41
@soutaro soutaro added this pull request to the merge queue Dec 19, 2024
Merged via the queue into ruby:master with commit da70891 Dec 19, 2024
19 checks passed
@soutaro soutaro added the Released PRs already included in the released version label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

3 participants