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

Set sub names when calling defer_sub #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

autarch
Copy link
Member

@autarch autarch commented Jan 27, 2017

I was trying to fix https://rt.cpan.org/Ticket/Display.html?id=119534. This doesn't fix it, but I think these changes are still worth making. AFAICT, Sub::Defer really wants us to pass in a sub name.

@karenetheridge karenetheridge self-assigned this Jan 27, 2017
@autarch
Copy link
Member Author

autarch commented Jan 29, 2017

The test failures are a bit confusing. The "changes doesn't have content" failure is sort of a false failure, since the Changes files doesn't have a listing for this version yet, right? But somehow it is still checking for release notes for 0.50. My EnsureChangesHasContent plugin would get this right, I think ;)

The 12_wrapper-definition.t failure is very confusing. It's some sort of heisenfailure. I can reproduce it by running the code multiple times. I think it might have to do with the fact that the test code uses ->can to get a coderef to the to_X sub. Then Sub::Defer replaces that sub by name, but the coderef itself still points to the old deferred sub. I don't understand why it sometimes works though.

@autarch
Copy link
Member Author

autarch commented Jan 29, 2017

Actually, I think the 12 failure is something else, where the use of Sub::Defer simply doesn't work properly with the wrapper functionality, but somehow it accidentally works in some cases. I could use some help from someone who understands the internals of MX::Types a little better.

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