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

Upgrade To Factory Bot. #3

Open
wants to merge 2 commits into
base: v2.0
Choose a base branch
from
Open

Upgrade To Factory Bot. #3

wants to merge 2 commits into from

Conversation

braidn
Copy link

@braidn braidn commented Aug 17, 2018

Let's get away from Factory Girl completely. This is a bit of a headache
but since we are using fork, bake this in.

Let's get away from Factory Girl completely. This is a bit of a headache
but since we are using fork, bake this in.
@braidn braidn self-assigned this Aug 17, 2018
Factory Bot 5 will no longer or soon no longer allow developers to
define attributes the older way. This removes a heap of deprecation
warnings.
@braidn
Copy link
Author

braidn commented Aug 20, 2018

This is confirmed working using this version of Solidus locally to assist a the Rails 5 upgrade branch:
https://github.com/glossier/glossier-v2/pull/4860

@braidn braidn requested review from DanielWright and ktheory August 20, 2018 20:28
@ktheory
Copy link

ktheory commented Aug 21, 2018

since we are using fork, bake this in.

Personally, I see a near-term goal of getting off our solidus fork and back onto the upstream gem, probably even before upgrading. We can maintain the Kernel.silence_warnings block around loading Solidus factories until we're on mainline and upgraded.

That is to say, I'm not sure we'd use this patch.

@braidn
Copy link
Author

braidn commented Aug 21, 2018

This is for the Rails 5 upgrade I started at the end of last week Aaron. Sorry I wasn’t more clear with this. For the spec suite to even load on that branch we need to rid ourselves completely of the FactoryGirl namespacing.

This branch should merge with the 2.0 branch, not the 1.4 version we use day to day now

@ktheory ktheory removed their request for review October 25, 2018 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants