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

Switch order of arguments in respond_to predicate #56

Merged
merged 1 commit into from
May 5, 2019

Conversation

waiting-for-dev
Copy link
Member

This is the order that dry-types expect, and it makes more sense
having function currying in mind

@solnic this should be merged to master. It was my mistake yesterday.

References #55

@solnic
Copy link
Member

solnic commented May 5, 2019

Ugh of course. I missed that too 😄 The build failed though. Maybe you gotta undef :respond_to? there.

@waiting-for-dev
Copy link
Member Author

Hmm it doesn't have nothing to do with respond_to? predicate. Am I wrong?

  1) Dry::Logic::Rule arity specialization arbitrary arity generates correct arity
     Failure/Error: expect(rule.method(:call).arity).to be(arity - curried)
     
       expected #<Integer:1> => 0
            got #<Integer:-1> => -1
     
       Compared using equal?, which compares object identity,
       but expected and actual are not the same object. Use
       `expect(actual).to eq(expected)` if you don't care about
       object identity in this example.
     # ./spec/unit/rule_spec.rb:190:in `block (4 levels) in <top (required)>'
Finished in 0.11962 seconds (files took 0.27258 seconds to load)
206 examples, 1 failure
Failed examples:
rspec ./spec/unit/rule_spec.rb:189 # Dry::Logic::Rule arity specialization arbitrary arity generates correct arity

@waiting-for-dev
Copy link
Member Author

waiting-for-dev commented May 5, 2019

Yep, this fails when rand(20) returns 0, also removing respond_to predicate:

    describe 'arbitrary arity' do
      let(:arity) { rand(20) }
      let(:curried) { arity.zero? ? 0 : rand(arity) }

      let(:options) { { args: [1] * curried, arity: arity } }
      let(:predicate) { double(:predicate) }

      it 'generates correct arity' do
        expect(rule.method(:call).arity).to be(arity - curried)
        expect(rule.method(:[]).arity).to be(arity - curried)
      end
    end

Anyway, if you want I can tackle it in another PR.

@waiting-for-dev
Copy link
Member Author

Polluting this PR but.. does it make sense an arity of 0 there? Couldn't we just rand(1..20)?

@solnic
Copy link
Member

solnic commented May 5, 2019

does it make sense an arity of 0 there? Couldn't we just rand(1..20)?

it does not, this spec needs to be fixed

Anyway, if you want I can tackle it in another PR.

Please do

@waiting-for-dev
Copy link
Member Author

@solnic done in #57

This is the order that `dry-types` expect, and it makes more sense
having function currying in mind
@waiting-for-dev waiting-for-dev force-pushed the respond_to_predicate branch from b0855c2 to 9687558 Compare May 5, 2019 08:17
@waiting-for-dev
Copy link
Member Author

Ok, I rebased it. Build clean 😄

@solnic solnic merged commit 8e54e8e into dry-rb:master May 5, 2019
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.

2 participants