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.

Sign up for GitHub

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

Jump to bottom

Replace factory girl with factory bot #1243

Merged
merged 2 commits into from Dec 20, 2017
Merged

Replace factory girl with factory bot #1243

merged 2 commits into from Dec 20, 2017

Conversation

rdunlop
Copy link
Contributor

@rdunlop rdunlop commented Oct 30, 2017

No description provided.

@rdunlop rdunlop requested review from jhanggi and meara October 30, 2017 18:12
@meara
Copy link
Contributor

meara commented Oct 30, 2017

@rdunlop can you leave 11f66dc out of this PR, and also, do you mind fixing the codeclimate warnings if you're doing this? They look like mostly alignment issues.

meara
meara suggested changes Oct 30, 2017
View reviewed changes
Copy link
Contributor

@meara meara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks like there's a few FactoryGirl references still around somewhere causing test failures

@rdunlop rdunlop force-pushed the replace_factory_girl_with_factory_bot branch from 23924e1 to 2b2ed9b Compare December 13, 2017 22:07
@rdunlop
Copy link
Contributor Author

rdunlop commented Dec 13, 2017

Updated @meara

jhanggi
jhanggi approved these changes Dec 13, 2017
View reviewed changes
Copy link
Contributor

@jhanggi jhanggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, @rdunlop. I'll create branches in the different schools with these changes. We'll probably have some school-specific changes. I'd like to have those changes ready to go when we merge this downstream.

So don't merge this just yet.

jhanggi
jhanggi approved these changes Dec 19, 2017
View reviewed changes
Copy link
Contributor

@jhanggi jhanggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to merge and start moving downstream.

@meara meara merged commit f62c1b7 into master Dec 20, 2017
@vandrijevik vandrijevik deleted the replace_factory_girl_with_factory_bot branch May 11, 2018 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhanggi jhanggi jhanggi approved these changes

@meara meara meara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rdunlop @meara @jhanggi