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

Rename predicate #respond_to into #responds_to #95

Closed
wants to merge 1 commit into from

Conversation

CandyFet
Copy link

@CandyFet CandyFet commented Mar 4, 2022

PR for issue 73

@CandyFet CandyFet requested a review from solnic as a code owner March 4, 2022 19:59
@solnic
Copy link
Member

solnic commented Mar 13, 2022

How about interface?(:foo) - it would be consistent with dry-types where we have Types.Interface(:foo)

@CandyFet
Copy link
Author

How about interface?(:foo) - it would be consistent with dry-types where we have Types.Interface(:foo)

I am not very familiar with dry gems yet. But i'll look deeply and will add code that ensures successful behaviour with Types.Interface

@dgutov
Copy link

dgutov commented Sep 27, 2022

@solnic Do you mean to suggest using interface? as the name instead of responds_to??

It might work (especially it it's going to accept multiple arguments). responds_to? is a much more obvious name, though.

@flash-gordon
Copy link
Member

IMO, this only adds confusion. The ruby convention uses the infinitive form (has_key? -> have_key?). Either interface? or method? would work I think

@solnic
Copy link
Member

solnic commented Sep 29, 2022

I'd be OK with either interface? or method? too.

@dgutov
Copy link

dgutov commented Sep 29, 2022

IMO, this only adds confusion. The ruby convention uses the infinitive form (has_key? -> have_key?)

I'm following the precedent made by includes?:

def includes?(value, input)
if input.respond_to?(:include?)
input.include?(value)
else
false
end
rescue TypeError
false
end

Either interface? or method? would work I think

I don't have a strong opinion (anything would be better than the status quo), but someone might mistake method? for implying the use of Module#method_defined?.

@solnic
Copy link
Member

solnic commented Oct 1, 2022

Ah yeah, I guess includes? wasn't a good choice either 😬

@dgutov
Copy link

dgutov commented Oct 1, 2022

At this point (two years later), I'd say any choice is good enough. As long as it's a change. :-)

Though I guess this particular PR might limit itself to adding a new predicate (in a minor version), for you to remove the obsolete one in the new major (?) version.

@solnic
Copy link
Member

solnic commented Oct 9, 2022

I'm gonna go with interface? because we already have Types.Interface in dry-types so this would be consistent.

solnic added a commit that referenced this pull request Oct 9, 2022
solnic added a commit that referenced this pull request Oct 9, 2022
@solnic
Copy link
Member

solnic commented Oct 14, 2022

Closing after all as this was done via #99

@solnic solnic closed this Oct 14, 2022
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.

4 participants