-
-
Notifications
You must be signed in to change notification settings - Fork 390
Update string/modulo_spec for ruby core changes #401
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
Thank you. |
core/string/modulo_spec.rb
Outdated
("foo%" % []).should == "foo%" | ||
it "raises an error if single % appears at the end" do | ||
lambda { ("%" % []) }.should raise_error(ArgumentError) | ||
lambda { ("foo%" % [])}.should raise_error(ArgumentError) | ||
end |
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.
If the change is merged, it will only applies to trunk
initially, and then maybe be backported.
Therefore, could you add a version guard like this
ruby_version_is ""..."2.5" do
old spec
end
ruby_version_is "2.5" do
new spec
end
?
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.
Matz said this behaviour is OK 👀 https://bugs.ruby-lang.org/issues/13315#note-4 |
Yeah! 🎉 |
* Add tests and specs. See ruby/spec#401. Patch by Yuta Iwama and Shintaro Morikawa. [ruby-core:80153] [Bug #13315] [Fix GH-1560] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@58890 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Merged in ruby/ruby@fd8df3a |
* Add tests and specs. See #401. Patch by Yuta Iwama and Shintaro Morikawa. [ruby-core:80153] [Bug 13315] [Fix ruby/ruby#1560] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@58890 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Thank you @eregon ! |
I will change
sprintf
to raise an error if single "%" character appears at the end.ruby/ruby#1560 (comment)
https://bugs.ruby-lang.org/issues/13315
This pull-request follows the change.
Please review.