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.

Sign up for GitHub

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

Jump to bottom

Support both customized namespace factories as well as default resolvable ones #1201

Merged
merged 1 commit into from
Apr 2, 2021
Merged

Support both customized namespace factories as well as default resolvable ones #1201

merged 1 commit into from
Apr 2, 2021

Conversation

wimski
Copy link
Contributor

@wimski wimski commented Apr 2, 2021 •
edited
Loading

Summary

In response to a discussion I started: #1196 (comment).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Code style has been fixed via composer fix-style

@wimski wimski mentioned this pull request Apr 2, 2021
Merged
4 tasks
@wimski wimski marked this pull request as ready for review April 2, 2021 11:46
@ahmed-aliraqi
Copy link
Contributor

@wimski Really I think using Factory::resolveFactoryName($modelName) is better, I agree with you 👍

mfn
mfn approved these changes Apr 2, 2021
View reviewed changes
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks legit to me and I'm onboard it it works now better for everyone!

@wimski could you please add a changelog? I imagine we might release this ASAP

@barryvdh 👍

@barryvdh barryvdh merged commit 2f61602 into barryvdh:master Apr 2, 2021
@wimski
Copy link
Contributor Author

wimski commented Apr 2, 2021

wimski could you please add a changelog? I imagine we might release this ASAP

Seems like it's already merged 🤷‍♂️

@mfn
Copy link
Collaborator

mfn commented Apr 2, 2021

no problem :) => #1203

@wimski wimski deleted the fix/models-factory branch April 3, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfn mfn mfn approved these changes

@barryvdh barryvdh barryvdh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wimski @ahmed-aliraqi @mfn @barryvdh