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

Fix version parsing #339

Merged
merged 2 commits into from Sep 24, 2014
Merged

Fix version parsing #339

merged 2 commits into from Sep 24, 2014

Conversation

haarg
Copy link
Member

@haarg haarg commented Sep 22, 2014

This fixes the version number parsing to handle underscores properly.

version.pm is too buggy to use for this.

Fixes #337

@haarg
Copy link
Member Author

haarg commented Sep 22, 2014

Ping @dagolden, @ribasushi, @Leont, @karenetheridge, @rjbs in case you have any input on this.

@oalders
Copy link
Member

oalders commented Sep 23, 2014

Also ping @shadowcat-mst, @miyagawa and @neilbowers. Anything to add?

@dagolden
Copy link

I'm only following this issue lightly, but I don't think version.pm's handling of underscores is well-advised, and historically did particularly perverse things depending on presence/absence of version.pm and the version of perl (see "Version Number Puzzles" for examples). And that's only for modules, not for distributions, where version numbers are irrelevant to PAUSE except for the presence/absence of underscores.

Generally speaking, the use of underscores in version numbers is a historical accident and cause more problems than it solves. Treating them as invisible for all purposes except deciding whether or not to index a distribution should solve more problems than it creates.

The Lyon 2014 "version number summit" agreed on this as the desired direction for handling of version numbers: https://gist.github.com/dagolden/9559280.

@Leont
Copy link

Leont commented Sep 23, 2014

I agree with @dagolden and @haarg, it's nothing less than crazy than a method called numify is not returning a valid number (worse yet, something that sort right when treated as one).

@karenetheridge
Copy link

I agree; let's stick to the decisions made at the version number summit (which needs a better name - what was the name of that restaurant?)

I'm also hoping that this will fix #284.

@miyagawa
Copy link

The current implementation of MetaCPAN's numification leads to issues like miyagawa/cpanminus#377 (comment) on cpanm. +1 on this patch.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling cfd63fc on haarg/version-parse-fix into 8fc9ffa on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling cfd63fc on haarg/version-parse-fix into 8fc9ffa on master.

oalders added a commit that referenced this pull request Sep 24, 2014
@oalders oalders merged commit 9868954 into master Sep 24, 2014
@oalders oalders deleted the haarg/version-parse-fix branch September 24, 2014 17:23
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.

version numification is incorrect
8 participants