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

Fixes #1236 Fixed version number for metacpan search #1241

Merged
merged 1 commit into from Nov 30, 2016

Conversation

@liger1978
Copy link
Contributor

liger1978 commented Nov 28, 2016

The new way of searching metacpan with an http POST consistently requires numerical version number (i.e. no v prefix). This patch tested against 73 CPAN modules: https://gitlab.com/harbottle/epmel/builds/6722395

@liger1978

This comment has been minimized.

Copy link
Contributor Author

liger1978 commented Nov 28, 2016

It's only the OSX build failing travis-ci (as usual)...

if cpan_version.nil?
self.version = metadata["version"]
self.version = "#{metadata["version"]}"

This comment has been minimized.

Copy link
@jordansissel

jordansissel Nov 30, 2016

Owner

this can just be self.version = metadata["version"] I think.

This comment has been minimized.

Copy link
@liger1978

liger1978 Nov 30, 2016

Author Contributor

It was just belt and braces to ensure self.version was a string before attempting the substitution below.

This comment has been minimized.

Copy link
@jordansissel

jordansissel Nov 30, 2016

Owner

Gotcha. My preferred way to do this is to call .to_s on the object: metadata["version"].to_s

No change required. Thanks for the explanation :)

@jordansissel

This comment has been minimized.

Copy link
Owner

jordansissel commented Nov 30, 2016

Lovely thank you! I made one comment for a style change, but it's minor and I am happy to merge.

@jordansissel jordansissel merged commit b560b0f into jordansissel:master Nov 30, 2016
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@jordansissel

This comment has been minimized.

Copy link
Owner

jordansissel commented Nov 30, 2016

@liger1978 how would you characterize this change, a bug fix? new feature?

@liger1978

This comment has been minimized.

Copy link
Contributor Author

liger1978 commented Nov 30, 2016

@jordansissel

This comment has been minimized.

Copy link
Owner

jordansissel commented Nov 30, 2016

@liger1978 ok cool, noted for the release notes <3

@jordansissel jordansissel added the bug label Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.