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

Test LWP has a Version sub/method #253

Open
wants to merge 1 commit into
base: master
from

Conversation

@JRaspass
Copy link
Contributor

@JRaspass JRaspass commented Apr 19, 2017

A simple test to make sure LWP->Version or LWP::Version() returns
the same as $LWP::VERSION.

A simple test to make sure LWP->Version or LWP::Version() returns
the same as $LWP::VERSION.
@coveralls
Copy link

@coveralls coveralls commented Apr 19, 2017

Coverage Status

Coverage increased (+0.07%) to 68.533% when pulling 0b95a5f on cv-library:test-version into 782e400 on libwww-perl:master.

@karenetheridge
Copy link
Member

@karenetheridge karenetheridge commented Apr 19, 2017

This Version sub is undocumented -- can we just get rid of it?

Its use is very limited:

http://grep.cpan.me/?q=LWP%3A%3AVersion
http://grep.cpan.me/?q=LWP-%3EVersion

@genio
Copy link
Member

@genio genio commented Apr 19, 2017

I would love to get rid of it. I'll see about putting in PRs to the few things making use of it later today.

@JRaspass
Copy link
Contributor Author

@JRaspass JRaspass commented Apr 20, 2017

Dropping any uses in favour of $LWP::VERSION, LWP::VERSION, or LWP->VERSION seems sane, since the coderef always exists and just returns the value of the package var.

@karenetheridge
Copy link
Member

@karenetheridge karenetheridge commented Apr 20, 2017

The VERSION() sub from UNIVERSAL does more than that -- when passed a value, it asserts that the current version satisfies the argument.

LWP->VERSION('1.0');  # returns true
LWP->VERSION('999');  # dies

This is commonly seen in code that conditionally requires a module for a feature:

if (eval { require Foo; Foo->VERSION('1.23') }) {
    ... # use the feature that was added in Foo 1.23...
}
@genio
Copy link
Member

@genio genio commented Apr 22, 2017

Our own bin scripts have been fixed.

  1. lwp-mirror
  2. lwp-request

Other dists doing the wrong thing...

  1. RT# 121280 for Crypt::NSS
  2. RT# 121281 for CPAN::SQLite
  3. RT# 121282 for WebFS::FileCopy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.