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

Add no_nonmoose option #22

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

Conversation

smith153
Copy link

When combined with use_moose, doesn't add MooseX::NonMoose to result classes

When combined with use_moose, doesn't add MooseX::NonMoose to result classes
@karenetheridge
Copy link
Contributor

Why would you want to do that? That would break DBIx::Class functionality.

@smith153
Copy link
Author

smith153 commented Apr 3, 2020

@karenetheridge Don't know, but was a requirement for an existing project.

@karenetheridge
Copy link
Contributor

Did you try setting use_moose to false?

@jbakerdev
Copy link

@karenetheridge @smith153 If the result classes inherit from a result_base_class that is already moosified, then there is no need for MooseX::NonMoose, no?

I moosified the base class in an attempt to silence warnings like this:

Not inlining 'new' for MyApp::Schema::Result::Thing since it is not inheriting the default Moose::Object::new
If you are certain you don't need to inline your constructor, specify inline_constructor => 0 in your call to MyApp::Schema::Result::Thing->meta->make_immutable

I thought I was being clever with this (instead of adding "inline_constructor => 0" to all my result classes) but then I remembered that I shouldn't be modifying the generated classes... and here I am...

Thoughts/suggestions?

@karenetheridge
Copy link
Contributor

f the result classes inherit from a result_base_class that is already moosified, then there is no need for MooseX::NonMoose, no?

Loader could be smarter and examine the base class(es) to see if MooseX::NonMoose is necessary. It looks like the code for that is at about line 2065 of DBIx/Class/Schema/Loader/Base.pm.

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.

None yet

3 participants