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 Sub::Defer but without the sub names #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oschwald
Copy link

@oschwald oschwald commented Jun 3, 2017

This is the same as #7 but with the commit for the sub names dropped. That commit causes test failures depending on load order.

This is to maintain compatibility with modules that declare types but
never use them. In 0.49, this could cause an exception if
Sub::Defer::undefer_all was called.
@oschwald
Copy link
Author

Ping. @autarch, any thoughts on this? See moose/MooseX-Types-LoadableClass#3 also.

@autarch
Copy link
Member

autarch commented Jun 12, 2017

I don't really have any thoughts. I'm not sure what's up with LoadableClass. I'm not using MX-Types in new code myself.

@oschwald
Copy link
Author

The PR I linked to fixed the particular issue in LoadableClass, which was declaring a type that was never actually defined. It instead replaced the declared version with the LoadableClass type. This caused issues for your original changes when undefer_all() was called as it would try to find the non-existent type at that point and croak if it could not be found. Delaying the croak seems to be the safest solution as other type packages may have similar issues.

@oschwald
Copy link
Author

Is there any chance this could get reviewed?

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.

2 participants