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

Transmoogrification - Moose => Moo conversion of HTTP::Throwable #5

Closed
wants to merge 11 commits into from

Conversation

felliott
Copy link
Contributor

Hello Ricardo,

This patchset switches HTTP::Throwable from Moose to Moo-based. It passes all of the current tests, and I've also tested it with the Moose-based subclassing in timbunce/WebAPI-DBIC. I've added dependecies on Type::Tiny, Types::Standard, Package::Variant, and namespace::clean. Please let me know if there's anything you would like me to change or fix. Thank you and have a great day!

Cheers,
Fitz

@felliott
Copy link
Contributor Author

Forced-push some minor fixes, thanks to Mithaldu++!

 * per haarg: Moo::Object is undocumented as part of the Moo API and
   shouldn't be relied upon.  Instead, only run the extends if the user
   overrides base_class.
@karenetheridge
Copy link
Contributor

Is this merge-ready? it would be great to no longer have to pay the Moose tax when used with Catalyst.

@jjn1056
Copy link

jjn1056 commented Sep 2, 2014

+1

@rjbs
Copy link
Owner

rjbs commented Jul 1, 2015

I will test this against my work code base uses of HTTP-Throwable and either release or review by next week. (It's on my calendar now!)

@rjbs
Copy link
Owner

rjbs commented Jul 11, 2015

I have tested my code against this, and it looks okay.

All of the roles had use Moose::Role replaced with use Moo::Role, which seem good, but then they had no Moo::Role replaced with … nothing. So the classes end up with with methods, etc.

I will fix this locally, but please keep that in mind next time you do a transmoogrification like this.

@rjbs
Copy link
Owner

rjbs commented Jul 11, 2015

I have uploaded HTTP-Throwable-0.021-TRIAL.tar.gz. I'll give this a while before making it the stable version.

@karenetheridge
Copy link
Contributor

So the classes end up with with methods, etc.

FWIW, I catch this via my dzil author bundle by including [Test::CleanNamespaces].

@rjbs
Copy link
Owner

rjbs commented Aug 3, 2015

This has been released as v0.022. Thank you for all your efforts!

@rjbs rjbs closed this Aug 3, 2015
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.

5 participants