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

[7.x] Remove Factory "types" #30867

Merged
merged 1 commit into from
Dec 18, 2019
Merged

[7.x] Remove Factory "types" #30867

merged 1 commit into from
Dec 18, 2019

Conversation

browner12
Copy link
Contributor

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.

- 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.
browner12 added a commit to browner12/docs that referenced this pull request Dec 17, 2019
do not merge this unless laravel/framework#30867 is merged.

I wanted to get this opened, though, so I wouldn't forget.
browner12 added a commit to browner12/docs that referenced this pull request Dec 17, 2019
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
Copy link
Member

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)) {
Copy link
Member

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)) {

Copy link
Contributor Author

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.

@taylorotwell taylorotwell merged commit 67b814b into laravel:master Dec 18, 2019
@browner12 browner12 deleted the remove-named-factories branch December 18, 2019 18:32
ibpavlov added a commit to ibpavlov/module-lumen that referenced this pull request Apr 7, 2021
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.
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.

3 participants