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

Using Package::Stash #2

Merged
merged 11 commits into from Sep 28, 2017

Conversation

Projects
None yet
3 participants
@yanick
Contributor

yanick commented Sep 6, 2017

mostly an exercise in Yak shaving for the CPAN challenge. :-)

The biggest change is the use of Package::Stash to play with the function overloading and the such.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 6, 2017

Coverage Status

Coverage decreased (-0.07%) to 99.048% when pulling 63a75e4 on yanick:package-stash into a181152 on DrHyde:master.

coveralls commented Sep 6, 2017

Coverage Status

Coverage decreased (-0.07%) to 99.048% when pulling 63a75e4 on yanick:package-stash into a181152 on DrHyde:master.

@DrHyde

This comment has been minimized.

Show comment
Hide comment
@DrHyde

DrHyde Sep 15, 2017

Owner

I like the look of this shaved yak! Unfortunately it has test failures. I thought it would be just something simple like adding Package::Stash and List::MoreUtils to the list of prereqs but alas it isn't.

Owner

DrHyde commented Sep 15, 2017

I like the look of this shaved yak! Unfortunately it has test failures. I thought it would be just something simple like adding Package::Stash and List::MoreUtils to the list of prereqs but alas it isn't.

@yanick

This comment has been minimized.

Show comment
Hide comment
@yanick

yanick Sep 28, 2017

Contributor

Hmm... I'll have a look at the travis failures and see if I can pinpoint what I messed up...

Contributor

yanick commented Sep 28, 2017

Hmm... I'll have a look at the travis failures and see if I can pinpoint what I messed up...

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 28, 2017

Coverage Status

Coverage decreased (-0.06%) to 99.057% when pulling dd30edc on yanick:package-stash into a181152 on DrHyde:master.

coveralls commented Sep 28, 2017

Coverage Status

Coverage decreased (-0.06%) to 99.057% when pulling dd30edc on yanick:package-stash into a181152 on DrHyde:master.

@yanick

This comment has been minimized.

Show comment
Hide comment
@yanick

yanick Sep 28, 2017

Contributor

Booyah! I think Travis is happy with my new change: https://travis-ci.org/DrHyde/perl-modules-Sub-WrapPackages/builds/280903524

For some reason, for a bunch of perls in-between the lines

$blah -~ s/(...).../.../;
Package::Stash->new($1);

the $1 was being clobbered. So I just saved it into a temporary variable, and all seems to be fine, now. :-)

Contributor

yanick commented Sep 28, 2017

Booyah! I think Travis is happy with my new change: https://travis-ci.org/DrHyde/perl-modules-Sub-WrapPackages/builds/280903524

For some reason, for a bunch of perls in-between the lines

$blah -~ s/(...).../.../;
Package::Stash->new($1);

the $1 was being clobbered. So I just saved it into a temporary variable, and all seems to be fine, now. :-)

@DrHyde DrHyde merged commit c1cbc8b into DrHyde:master Sep 28, 2017

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.06%) to 99.057%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DrHyde

This comment has been minimized.

Show comment
Hide comment
@DrHyde

DrHyde Sep 28, 2017

Owner

Thanks!

Owner

DrHyde commented Sep 28, 2017

Thanks!

@yanick

This comment has been minimized.

Show comment
Hide comment
@yanick

yanick Sep 28, 2017

Contributor

Pleasure was all mine, good sir. ^.^

Contributor

yanick commented Sep 28, 2017

Pleasure was all mine, good sir. ^.^

@DrHyde

This comment has been minimized.

Show comment
Hide comment
@DrHyde

DrHyde Oct 25, 2017

Owner

Unfortunately it introduces some problematical dependencies and has odd test failures on some older perls, so I'm reverting it. Hopefully I'll be able to cherry-pick at least some of the improvements though after doing that.

Owner

DrHyde commented Oct 25, 2017

Unfortunately it introduces some problematical dependencies and has odd test failures on some older perls, so I'm reverting it. Hopefully I'll be able to cherry-pick at least some of the improvements though after doing that.

@yanick

This comment has been minimized.

Show comment
Hide comment
@yanick

yanick Oct 29, 2017

Contributor

Unfortunately it introduces some problematical dependencies

Which ones, out of curiosity?

Hopefully I'll be able to cherry-pick at least some of the improvements

I think my commits were piecemealy enough to allow you to pick and choose. If not, feel free to ask me to rejuggle my stuff, and I'll do it gladly.

Contributor

yanick commented Oct 29, 2017

Unfortunately it introduces some problematical dependencies

Which ones, out of curiosity?

Hopefully I'll be able to cherry-pick at least some of the improvements

I think my commits were piecemealy enough to allow you to pick and choose. If not, feel free to ask me to rejuggle my stuff, and I'll do it gladly.

@DrHyde

This comment has been minimized.

Show comment
Hide comment
@DrHyde

DrHyde Oct 30, 2017

Owner

List::MoreUtils depends on XSLoader, the current version of which doesn't like early perl 5.8s, and earlier versions have bugs.

Package::Stash emits some just plain odd errors on early 5.8s that I couldn't replicate in a smaller chunk of code, so I suspect it's actually tickling a bug in perl itself (not clearing out a C variable or an overflow or something) that has since been fixed: when running the Sub::WrapPackages tests on early 5.8s Package::Stash spits out, eg, "Banana::Tree is not a module name ...". If you look at the Package::Stash source it looks like this might be related to that mysterious issue with $1 getting clobbered. But not only can't I replicate it in a smaller bit of code, I can't even reliably replicate it on all my machines - it blows up on Linux/ARM and OS X/Intel, but not Linux/Intel. Weird.

But yes, I did manage to pick out the bits that didn't rely on those modules, and they're in the version that I pushed out to the CPAN a few days ago. Thanks for your contribution!

Owner

DrHyde commented Oct 30, 2017

List::MoreUtils depends on XSLoader, the current version of which doesn't like early perl 5.8s, and earlier versions have bugs.

Package::Stash emits some just plain odd errors on early 5.8s that I couldn't replicate in a smaller chunk of code, so I suspect it's actually tickling a bug in perl itself (not clearing out a C variable or an overflow or something) that has since been fixed: when running the Sub::WrapPackages tests on early 5.8s Package::Stash spits out, eg, "Banana::Tree is not a module name ...". If you look at the Package::Stash source it looks like this might be related to that mysterious issue with $1 getting clobbered. But not only can't I replicate it in a smaller bit of code, I can't even reliably replicate it on all my machines - it blows up on Linux/ARM and OS X/Intel, but not Linux/Intel. Weird.

But yes, I did manage to pick out the bits that didn't rely on those modules, and they're in the version that I pushed out to the CPAN a few days ago. Thanks for your contribution!

@yanick

This comment has been minimized.

Show comment
Hide comment
@yanick

yanick Oct 30, 2017

Contributor

List::MoreUtils depends on XSLoader, the current version of which doesn't like early perl 5.8s, and earlier versions have bugs.

Aaaah, I see. And yeah, "early 5.8" indeed sounds like some very vintage versions. :-)

I did manage to pick out the bits that didn't rely on those modules

Beautiful. ^.^

Contributor

yanick commented Oct 30, 2017

List::MoreUtils depends on XSLoader, the current version of which doesn't like early perl 5.8s, and earlier versions have bugs.

Aaaah, I see. And yeah, "early 5.8" indeed sounds like some very vintage versions. :-)

I did manage to pick out the bits that didn't rely on those modules

Beautiful. ^.^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment