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

Added type checks for cpan authors #744

Merged
merged 1 commit into from Jul 30, 2014

Conversation

Projects
None yet
3 participants
@selfawaresoup

selfawaresoup commented Jul 30, 2014

CPAN 'authors' can be a list of strings, just a string or not set at all. This adds type checks to handle the different possibilities. Related to #712

self.vendor = metadata["author"].join(", ") unless metadata["author"].nil?
# author is not always set or it may be a string instead of an array
unless metadata["author"].nil?
if metadata["author"].respond_to?(:to_str)

This comment has been minimized.

@jordansissel

jordansissel Jul 30, 2014

Owner

I'd rather have this check if it's a String, instead of if it responds to :to_str; the contract (if there even is such a thing) on what to_s means vs to_str is not really clear in the docs and certainly not clear to me as a reader :P

I'll change this post-merge.

This comment has been minimized.

@selfawaresoup

selfawaresoup Jul 30, 2014

Sure, I actually wasn't sure if I should check for the type or the method. :)

This comment has been minimized.

@jordansissel

jordansissel Jul 30, 2014

Owner

I don't think there's any strict rule to play here, Ruby's a very flexible language in this respect. My personal habit is to either check for type or check for a method. Oddly, I've never seen this 'to_ary' and 'to_str' methods used in 7 years of ruby programming. Weird! :P

This comment has been minimized.

@jordansissel

jordansissel Jul 30, 2014

Owner

For my own learning -

Poking around String#to_str has really poor docs "returns the receiver" as does Array#to_ary which says "returns self".

Wondering what else has to_ary I asked for Enumerator#to_ary docs, and this method doesn't exist. Range#to_ary also does not exist.

Based on this, I think to_str and to_ary are weird, unclear, and we should not use them.

I'll merge as-is and switch to using type checks.

This comment has been minimized.

@loosi

loosi Jul 30, 2014

I do believe that in most cases multiple ways of getting the object type will work.
As for this...response_to? makes it (imho) more duck-type-like than just checking the type of object.
You dont really get advantages from respond_to? in this case.
Nevertheless to_str and to_ary are no methods, they are symbols for the respond_to? method.

jordansissel added a commit that referenced this pull request Jul 30, 2014

Merge pull request #744 from lnwdr/cpan-author-check
Added type checks for cpan authors

@jordansissel jordansissel merged commit cb698c5 into jordansissel:master Jul 30, 2014

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 18, 2014

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 27, 2014

jordansissel added a commit that referenced this pull request Apr 24, 2015

Merge pull request #744 from lnwdr/cpan-author-check
Added type checks for cpan authors

jordansissel added a commit that referenced this pull request Jun 20, 2016

Merge pull request #744 from lnwdr/cpan-author-check
Added type checks for cpan authors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment