Skip to content

Commit

Permalink
shim ruby2_keywords for <= 2.6
Browse files Browse the repository at this point in the history
  • Loading branch information
@malcolmohare
malcolmohare committed Feb 25, 2024
1 parent c0ea86c commit 869067e
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions spec/rspec/support/method_signature_verifier_spec.rb
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'rspec/support'
require 'rspec/support/method_signature_verifier'

def ruby2_keywords(*); end unless respond_to?(:ruby2_keywords, true)

This comment has been minimized.

Sign in to view

Copy link
@pirj

pirj Feb 25, 2024

Member

This would work for specs, but what if you run rspec on 2.5 with some other project?
We usually
ruby_keywords … if Onjectmethod? …

This comment has been minimized.

Sign in to view

Copy link
@malcolmohare

malcolmohare Feb 25, 2024

Author Contributor

I don't understand the concern. This shim is purely for rspec to not complain about the ruby2_keyword args on the methods in the spec file.

The actual MethodSignatureVerifier doesn't care about that method annotation, its using Hash.ruby2_keywords_hash? and since that call is gated by a flag which is only true for Ruby 3 or greater its not a problem for old versions.

This comment has been minimized.

Sign in to view

Copy link
@pirj

pirj Feb 25, 2024

Member

If we removed the if defined? from the MSV class, would any of specs fail on earlier Tuby where ruby2_keywords are not defined?

This comment has been minimized.

Sign in to view

Copy link
@malcolmohare

malcolmohare via email Feb 25, 2024

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@pirj

pirj Feb 25, 2024

Member

Yes.

Another concern is that ruby2_keywords changes what we expect in specs. We used to pass kwargs to eg valid?, and previously it didn’t add that kwargs flag to the last hash arg, but now it does. Does this mean we’ve altered all the spec where valid? is used?
I’d say if we need to use a valid?-like method in some example that needs the last arg to be flagged as kwargs, let’s add another valid_kw? method just for this purpose.

I admit that I may be overly cautious with this, and the above may not make much sense. Please disregard my rant in this case.

This comment has been minimized.

Sign in to view

Copy link
@JonRowe

JonRowe Feb 26, 2024

Member

This shim would cause other peoples code to pass a spec suite when using this method, but then fail in reality, we try very hard to avoid polluting the environment in this fashion.

This comment has been minimized.

Sign in to view

Copy link
@malcolmohare

malcolmohare Feb 26, 2024

Author Contributor

Got it, I've removed that and instead made two declarations of the functions that needed ruby2_keywords in the spec depending on the code version. I wasn't aware that the shim applied outside of the file it was put in.

module RSpec
module Support
RSpec.describe 'verifying methods' do
Expand Down

0 comments on commit 869067e

Please sign in to comment.