-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Support uuids in build_stubbed #1583
Conversation
@mike-burns This looks good. Can you take a look at it? |
Right, and we're going to need this now that we've outlawed setting the The thing I've been staring at all afternoon, though, is: this is all very ActiveRecord-specific, which means it should go in OK, I think the plan is: merge this PR, then work on moving the stub strategy into f_b_rails. That will require a lot of finesse and delicacy because I don't want to break any existing code in the process. Thanks for listening as I talk this through. Hey @jaynetics @alexandreruban and @StefSchenkelaars , thank you all for the work you put into this! |
(Current status: figuring out why CI isn't running. sigh) |
@mike-burns might be related #1593 |
Thanks for being on top of things, @MatheusRich! But if that's the case, why aren't those CI checks running? |
I pushed this branch to the main repo and watched CI pass: https://github.com/thoughtbot/factory_bot/actions/runs/6592716225 Going to merge bypassing CI for this PR. |
@mike-burns thanks for merging!
if you want to support either way, |
Yeah fair - perhaps the actual original problem was splitting out a Rails-specific gem to begin with, with all the vagueness that "Rails" implies. |
This is based on #1514 and fixes #1498.
The only things i did, compared to #1514, are the following:
The relevant rails API hasn't changed since PR #1514 was created 2 years ago. It would be nice to finally get support for UUIDs. 😊