-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.6] Added states to model factory after callbacks #23551
[5.6] Added states to model factory after callbacks #23551
Conversation
Seems OK to me. Can you add tests? |
Yes I can. I am traveling currently and will be back in a few days. I will finish it up then. I just wanted to get it out there quick while it was being discussed. |
I'm assuming the most recent push was a mistake, correct? lots of unrelated changes |
ugh yes. I was merging from upstream why did it do that? and how do I fix it? Now that #23495 has been merged, my proposed changes will have been breaking. I assume that is not OK and have updated the code accordingly although I am disappointed that the api is less consistent because of it. |
@unstoppablecarl I would make a hard reset on the feature branch, and then push to github using the --force flag |
41dbc74
to
350ea24
Compare
There are some existing areas in |
keep this PR simple. if the tests are not related to this PR, save those change for another PR |
A docs PR for this would be great. |
What's the best way to get a Faker instance in callbacks? |
It gives the faker instance in the second argument of the callback. |
This is an addition to this recently merged PR made by @Indemnity83.
#23495
It adds the ability to bind the after callbacks to specific model factory states using the existing api pattern of existing create/make methods.
I this is not completely ready for PR. I wanted to submit it quickly after the above PR was merged to keep the discussion going. It still needs tests. Tagging the people that discussed the above PR to get their input: @deleugpn @browner12 @shadoWalker89