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

Replace Any::Moose with plain Moose #11

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Dec 21, 2015

Any::Moose is deprecated and thus needs to be replaced. RT#104920
recommends replacing Any::Moose with Moo (since Moo provides a large
subset of the Moose functionality but with a shorter startup time and
lower overhead) however in order to get the enum type constraints working
properly it turned out to be much simpler to simply replace Any::Moose
with Moose (it was a simple drop-in replacement). The tests still pass
and it doesn't seem to be any slower than before. According to
http://shadow.cat/blog/matt-s-trout/moo-versus-any-moose/ it seems that
Any::Moose could still load Moose anyway (depending upon configuration),
so the replacement implemented here seems like a sensible solution and
resolves the issue in RT#104920.

Replace Any::Moose with plain Moose
`Any::Moose` is deprecated and thus needs to be replaced.  RT#104920
recommends replacing `Any::Moose` with `Moo` (since `Moo` provides a large
subset of the `Moose` functionality but with a shorter startup time and
lower overhead) however in order to get the enum type constraints working
properly it turned out to be much simpler to simply replace `Any::Moose`
with `Moose` (it was a simple drop-in replacement).  The tests still pass
and it doesn't seem to be any slower than before.  According to
http://shadow.cat/blog/matt-s-trout/moo-versus-any-moose/ it seems that
`Any::Moose` could still load `Moose` anyway (depending upon configuration),
so the replacement implemented here seems like a sensible solution and
resolves the issue in RT#104920.
@mstratman

This comment has been minimized.

Show comment
Hide comment
@mstratman

mstratman Dec 21, 2015

Owner

Hi Paul,
Thanks for doing all this great work on the module. Unfortunately I haven't had much time to go through it in detail yet, but I love what I'm seeing. Let me know if you think any of these are time sensitive and I'll merge and release, otherwise I'd love to dig in and understand the changes, as well as apply them to the rest of the Barcode and HTML::Barcode modules, and plan to make time to do so next week.

Regarding this pull in particular, do you have any opinion on whether Type::Tiny + Moo might give us a lighter weight way to achieve the same goal? Let me know what you think.

Thanks again!

Owner

mstratman commented Dec 21, 2015

Hi Paul,
Thanks for doing all this great work on the module. Unfortunately I haven't had much time to go through it in detail yet, but I love what I'm seeing. Let me know if you think any of these are time sensitive and I'll merge and release, otherwise I'd love to dig in and understand the changes, as well as apply them to the rest of the Barcode and HTML::Barcode modules, and plan to make time to do so next week.

Regarding this pull in particular, do you have any opinion on whether Type::Tiny + Moo might give us a lighter weight way to achieve the same goal? Let me know what you think.

Thanks again!

@paultcochrane

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Dec 22, 2015

Contributor

Hi Mark,
thanks for replying so quickly! None of the changes are really time sensitive, although as I submit more PRs they're likely to overlap and cause conflicts (in which case, just ping me and I'll resolve the conflicts and resubmit). They're submitted in the hope that they help, so there's no pressure on you to merge them quickly (or at all if you don't wish!) :-)

WRT this PR: I tried a combination of Type::Tiny and Moo, however it seemed that there were too many code changes required. That's what I liked about the Moose change: it was a simple drop-in replacement. However, I can understand the wish to use something lighter-weight. I can try getting a solution to work using Type::Tiny and Moo and submit that as a separate PR and you can decide which solution you'd rather have.

Cheers,
Paul

Contributor

paultcochrane commented Dec 22, 2015

Hi Mark,
thanks for replying so quickly! None of the changes are really time sensitive, although as I submit more PRs they're likely to overlap and cause conflicts (in which case, just ping me and I'll resolve the conflicts and resubmit). They're submitted in the hope that they help, so there's no pressure on you to merge them quickly (or at all if you don't wish!) :-)

WRT this PR: I tried a combination of Type::Tiny and Moo, however it seemed that there were too many code changes required. That's what I liked about the Moose change: it was a simple drop-in replacement. However, I can understand the wish to use something lighter-weight. I can try getting a solution to work using Type::Tiny and Moo and submit that as a separate PR and you can decide which solution you'd rather have.

Cheers,
Paul

@mstratman

This comment has been minimized.

Show comment
Hide comment
@mstratman

mstratman Dec 31, 2015

Owner

Closing in favor of #13 but let me know if you prefer otherwise.

Owner

mstratman commented Dec 31, 2015

Closing in favor of #13 but let me know if you prefer otherwise.

@mstratman mstratman closed this Dec 31, 2015

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