-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[7.x] Remove Factory "types" #30867
[7.x] Remove Factory "types" #30867
Conversation
- simplify the global helper `factory()`. since the signature is no longer dynamic, we can be a little more explicit in our parameters - remove `defineAs`, `createAs`, `makeAs`, and `rawOf` which were methods specific to factory "types" - update the `FactoryBuilder` to remove references to the "name". we do keep one reference to the name "default" because that is how the callbacks are registered.
do not merge this unless laravel/framework#30867 is merged. I wanted to get this opened, though, so I wouldn't forget.
do not merge this unless laravel/framework#30867 is merged. I wanted to get this opened, though, so I wouldn't forget.
* | ||
* @param dynamic class|class,name|class,amount|class,name,amount | ||
* @param string $class | ||
* @param int $amount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int|null
return $factory->of($arguments[0], $arguments[1])->times($arguments[2] ?? null); | ||
} elseif (isset($arguments[1])) { | ||
return $factory->of($arguments[0])->times($arguments[1]); | ||
if (isset($amount) && is_int($amount)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be just if (! is_null(amount)) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yah, i went back and forth on this. looks like it's merged already, so feel to submit a PR.
With illuminate/database version 7 Factory names are removed - more info laravel/framework#30867 Checking for property `name` ensures that this parameter can be passed.
Way back in October 2016, I created the factory "states" feature, and added documentation for it.
#14241
https://github.com/laravel/docs/pull/2735/files
A couple days later Taylor removed all reference to factory "types" from the documentation.
laravel/docs@e38e88b#diff-1620181c4412a72179f78bc2be6c6e68
Going on 3 years here I think we're good to remove the old functionality from the FW in the next major.
It appears the feature was never tested, so everything is still green with no changes to the tests.