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

CPAN Pull Request Challenge - Very small test improvements! #1

Merged
merged 2 commits into from Jan 12, 2015

Conversation

Projects
None yet
2 participants
@kaoru
Contributor

kaoru commented Jan 6, 2015

Hi Stevan,

I got catalyst-view-excel-template-plus as my CPAN Pull Request Challenge distribution for January. Seems like it's a pretty small module so I only found a couple of small changes on my first look-over.

When running the tests I did get a very loud stack trace from Email::Template::Plus

Class::MOP::load_class is deprecated at /home/alex/perl5/perlbrew/perls/perl-5.20.1/lib/site_perl/5.20.1/x86_64-linux/Class/MOP.pm line 69.
    Class::MOP::load_class("Excel::Template::Plus::TT") called at /home/alex/perl5/perlbrew/perls/perl-5.20.1/lib/site_perl/5.20.1/Excel/Template/Plus.pm line 14
    eval {...} called at /home/alex/perl5/perlbrew/perls/perl-5.20.1/lib/site_perl/5.20.1/Excel/Template/Plus.pm line 14
...
    Catalyst::Test::_local_request("TestApp", "http://localhost/test_one") called at /home/alex/perl5/perlbrew/perls/perl-5.20.1/lib/site_perl/5.20.1/Catalyst/Test.pm line 32
    Catalyst::Test::__ANON__("http://localhost/test_one") called at t/001_basic.t line 20

As far as I can tell you've had your fingers in the pies of Email::Template::Plus and Class::MOP at one time or another so perhaps you could update Email::Template::Plus::TT to use whatever the correct alternative to Class::MOP::load_class() is?

If there are any other changes or feature requests you had in mind for catalyst-view-excel-template-plus let me know and I'll see if I can put in a slightly more impressive pull request before the end of January!

Thanks,

Alex

kaoru added some commits Jan 6, 2015

Correct use of "no_plan" in t/000_load.t
The previous code 'use Test::More no_plan => 1;' warned at compile time:

    'Unknown option: 1'
@stevan

This comment has been minimized.

Show comment
Hide comment
@stevan

stevan Jan 6, 2015

Owner

@kaoru excellent, I will likely review this by the weekend, merge it and release. If you are so inclined you could patch Excel::Template::Plus yourself, the repo is here -> https://github.com/stevan/excel-template-plus, if not, don't worry, I will do it when I handle this patch.

Thanks!

Owner

stevan commented Jan 6, 2015

@kaoru excellent, I will likely review this by the weekend, merge it and release. If you are so inclined you could patch Excel::Template::Plus yourself, the repo is here -> https://github.com/stevan/excel-template-plus, if not, don't worry, I will do it when I handle this patch.

Thanks!

@kaoru

This comment has been minimized.

Show comment
Hide comment
@kaoru

kaoru Jan 10, 2015

Contributor

Did that PR for Excel::Template::Plus - stevan/excel-template-plus#1

Contributor

kaoru commented Jan 10, 2015

Did that PR for Excel::Template::Plus - stevan/excel-template-plus#1

stevan added a commit that referenced this pull request Jan 12, 2015

Merge pull request #1 from kaoru/test-improvements
CPAN Pull Request Challenge - Very small test improvements!

@stevan stevan merged commit 81032ec into stevan:master Jan 12, 2015

@stevan

This comment has been minimized.

Show comment
Hide comment
@stevan

stevan Jan 12, 2015

Owner

Thanks for the patch, will push a release sometime this week!

Owner

stevan commented Jan 12, 2015

Thanks for the patch, will push a release sometime this week!

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