The ruby/spec on Ruby 2.3 check is failing in this PR - I'm not sure what the existing conventions are for adding specs for new features like this.
Should the new tests be wrapped in unless RUBY_VERSION <= "2.3" conditions?
Or should the new tests be added somewhere other than /spec instead?
Or some other convention? Please advise!
Yes, in such a case you need to use ruby_version_is guards as documented on https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#guards,
because the behavior of previous versions will not change (unless this is decided to be backported, but very unlikely for a new feature, and even then the backport would be done later so the guards would be needed at first).
It is not backward compatible.
And I'm not sure why we need to change just #tap and not other methods.
There are some discussions going on about passing arguments whith Symbol#to_proc shorthand: #12115, #15301, etc. Personally I'd rather see one of these implemented. It would also fix this issue.
Eregon (Benoit Daloze) wrote:
Yes, in such a case you need to use ruby_version_is guards as documented on https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#guards,
because the behavior of previous versions will not change (unless this is decided to be backported, but very unlikely for a new feature, and even then the backport would be done later so the guards would be needed at first).
oleynikov (Alexander Oleynikov) wrote:
And I'm not sure why we need to change just #tap and not other methods.
There are some discussions going on about passing arguments whith Symbol#to_proc shorthand: #12115, #15301, etc. Personally I'd rather see one of these implemented. It would also fix this issue.
Thanks for the links to the other discussions @oleynikov, pretty interesting, I'll keep an eye on those as well!
For Kernel#tap specifically I think it still makes sense to allow it to accept arguments like Kernel#send since they basically behave the same aside from tap always returning self. Relying on the proposals to support Symbol#to_proc with arguments (which could take awhile until they're actually merged) looks a bit more abstract than it needs to be:
array.tap(:delete,object)# vsarray.tap(&:delete.(object))# if that's the syntax they agree on# vsarray.tap{|a|a.delete(object)}# currently supported