Skip to content

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Nov 4, 2025

Partially revert #168 because strscan_rest_p did not have rb_warning("StringScanner#rest? is obsolete").

It is actively used by the latest tzinfo.gem, and we shouldn't remove it without deprecating it.

Partially revert ruby#168 because
strscan_rest_p did not have rb_warning("StringScanner#rest? is obsolete").

It is actively used by the latest tzinfo.gem, and we shouldn't remove it
without deprecating it.
@k0kubun k0kubun merged commit f3fdf21 into ruby:master Nov 4, 2025
38 checks passed
@k0kubun k0kubun deleted the strscan-rest-p branch November 4, 2025 21:33
matzbot pushed a commit to ruby/ruby that referenced this pull request Nov 4, 2025
(ruby/strscan#169)

Partially revert ruby/strscan#168 because
strscan_rest_p did not have `rb_warning("StringScanner#rest? is
obsolete")`.

It is actively used by the latest tzinfo.gem, and we shouldn't remove it
without deprecating it.

ruby/strscan@f3fdf21189
@nobu
Copy link
Member

nobu commented Nov 5, 2025

That method is documented as obsolete since a6c2a8e at 2004-02-18.
Is it still used?

@kou
Copy link
Member

kou commented Nov 5, 2025

#168 (comment)

@kou
Copy link
Member

kou commented Nov 5, 2025

How about adding explicit rb_warning() before we remove it?

@k0kubun
Copy link
Member Author

k0kubun commented Nov 5, 2025

Yeah, that's my point. I'm suggesting to ship a release with rb_warning() for it first if we want to remove it. As of a6c2a8e, other methods like #empty? had rb_warning(), but #rest? didn't. So it has been a bit hard for users to notice it's an obsolete method, even though its documentation has said so for 21 years.

jacob-shops pushed a commit to Shopify/ruby that referenced this pull request Nov 12, 2025
(ruby/strscan#169)

Partially revert ruby/strscan#168 because
strscan_rest_p did not have `rb_warning("StringScanner#rest? is
obsolete")`.

It is actively used by the latest tzinfo.gem, and we shouldn't remove it
without deprecating it.

ruby/strscan@f3fdf21189
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants