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

Remove Kernel#trust, #unstrust and #untrusted? #1024

Closed
wants to merge 2 commits into from

Conversation

AI-Mozi
Copy link
Contributor

@AI-Mozi AI-Mozi commented May 12, 2023

#1016
Feature #16131

The following deprecated methods are removed.
[ ] Kernel#trust, Kernel#untrust, Kernel#untrusted?
[Feature #16131]

Idk if this is the correct way to write removed specs and also I had a problem with untrusted? error message. It won't mach with "undefined method `untrusted?' for".

@AI-Mozi AI-Mozi marked this pull request as ready for review May 12, 2023 08:54
@headius
Copy link
Contributor

headius commented May 12, 2023

The policy in the past has been to avoid writing specs for things that just aren't there, because obviously there's an infinite number of things that aren't there. I don't know what policy we want to have going forward. Everyone's excited about removing these methods but I don't know if we need to test for that condition.

@AI-Mozi
Copy link
Contributor Author

AI-Mozi commented May 12, 2023

It makes sense, I'll just close this pr then, thanks.

@AI-Mozi AI-Mozi closed this May 12, 2023
@andrykonchin
Copy link
Member

andrykonchin commented May 12, 2023

The only change (for all the removed methods) that might be appropriate is to fix version guards for deprecation warnings and replace ""..."3.0" with ""..."3.2" as far as only in Ruby 3.2 behaviour changed and warnings were still printed in Ruby 3.1.

Just for the record, the commit removing the methods is ruby/ruby@39bc5de

@eregon
Copy link
Member

eregon commented May 14, 2023

I think it could be worthwhile to test this (with respond_to?), otherwise other Ruby implementations might keep these methods longer than they should.
OTOH there is already one way that should lead to remove these methods: the NEWS of 3.2 mention this removal (e.g. oracle/truffleruby#3039).
So indeed it seem unnecessary.

(there is also https://github.com/oracle/truffleruby/blob/master/spec/truffle/methods_spec.rb but that's currently truffleruby-specific)

@eregon
Copy link
Member

eregon commented May 20, 2023

FYI the specs with respond_to? were added in #1031, I think it makes sense and it's natural to add such specs when following the NEWS file from 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants