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

Moo-sify ALL THE THINGS #44

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

Moo-sify ALL THE THINGS #44

wants to merge 1 commit into from

Conversation

yanick
Copy link
Collaborator

@yanick yanick commented Aug 18, 2016

To be fair, I am not sure if this a wonderful idea or an exercise in yak shaving. In any case, here it is for your potential pleasure. The tests are passing for me, and they don't take much more time from the original raw-object code; time went from 5 seconds to 6, and to 11 if I change Moo for Moose. :-(

@stevan
Copy link
Owner

stevan commented Aug 18, 2016

Question, does this scratch any particular itch for you? Because to be honest, I don't think that Moo(se)ifying this code provides any real benefits. I am always open to discussion though.

@yanick
Copy link
Collaborator Author

yanick commented Aug 18, 2016

No real itch, beyond "oooh, it would look 0.67% cleaner if the class stuff was Moobstracted". And I totally agree that for code of that size, that's probably compulsive shaving more than anything else. So, yeah. you are totally justified to go "thanks but, yeeeeah, no thanks". I just had to do it, the voices didn't give me any other choice. :-)

@yanick
Copy link
Collaborator Author

yanick commented Aug 19, 2016

Actually, all that stuff can actually be useful for one thing: the new Promises::Role::Promise role can be used to represent a generic promise interface. If it was around, I could write a Future::AsPromise interface and have code that can accept either implementation by only caring about ->DOES('Promises::Role::Promise').

hmmm, but then Promises::Role::Promise should probably be on a distro of its own (Role::Promise) so that it can be used by either Future or Promises without the need to install both.

... sorry, my mad scientist button is kinda stuck this week. I'll be silent, now, I swear. ;-)

@stevan
Copy link
Owner

stevan commented Aug 19, 2016

Promises::Role::Promise was my favorite part actually, so I am happy to hear you will preserve it

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

Successfully merging this pull request may close these issues.

None yet

2 participants