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

Replace Class::Load by Module::Runtime #5

Closed
wants to merge 1 commit into from

Conversation

@choroba
Copy link

@choroba choroba commented Sep 22, 2018

Fixes #3.

Replacing Class::Load::load_class with Module::Runtime::use_module
doesn't work for classes loaded from files that don't correspond to
their names (Some::Foo in t/basic.t). Using use_package_optimistically
instead makes the tests pass, but according to the documentation of
Module::Runtime, if a file with the expected name was placed into
@inc, it would be loaded despite the fact the package had already been
loaded from a different file, which is a behaviour change. There's
nothing like Class::Load::is_class_loaded in Module::Runtime.

Fixes #3.

Replacing Class::Load::load_class with Module::Runtime::use_module
doesn't work for classes loaded from files that don't correspond to
their names (Some::Foo in t/basic.t). Using use_package_optimistically
instead makes the tests pass, but according to the documentation of
Module::Runtime, if a file with the expected name was placed into
@inc, it would be loaded despite the fact the package had already been
loaded from a different file, which is a behaviour change. There's
nothing like Class::Load::is_class_loaded in Module::Runtime.
@choroba
Copy link
Author

@choroba choroba commented Sep 22, 2018

This PR is part of the CPAN Pull Request Challenge. I'm not sure I opened the PR against the correct branch, so please double-check.

@yanick yanick self-requested a review Sep 25, 2018
@yanick yanick added this to the v1.01 milestone Sep 25, 2018
@yanick
Copy link
Owner

@yanick yanick commented Sep 26, 2018

Merged and deployed, thanks!

@yanick yanick closed this Sep 26, 2018
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

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