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

Replace 'girl' with 'bot' everywhere #1051

Merged
merged 2 commits into from Oct 20, 2017

Conversation

Projects
None yet
7 participants
@Euraldius
Contributor

Euraldius commented Oct 20, 2017

Also: add a deprecation warning to factory_girl, asking users to switch to factory_bot

#921

Replace 'girl' with 'bot' everywhere
Also: add a deprecation warning to factory_girl, asking users to switch to
factory_bot

#921
allow(factory).to receive(:compile)
allow(factory).to receive(:run)
end
it "runs the factory with the correct overrides" do
association_named(:author, strategy: :build, great: "value")
expect(factory).to have_received(:run).with(factory_girl_strategy_name, { great: "value" })
expect(factory).to have_received(:run).with(factory_bot_strategy_name, { great: "value" })

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Redundant curly braces around a hash parameter.
Line is too long. [94/80]

@houndci-bot

houndci-bot Oct 20, 2017

Redundant curly braces around a hash parameter.
Line is too long. [94/80]

let(:factory) { double("associate_factory") }
def association_named(name, overrides)
runner = FactoryGirl::FactoryRunner.new(name, overrides[:strategy], [overrides.except(:strategy)])
runner = FactoryBot::FactoryRunner.new(name, overrides[:strategy], [overrides.except(:strategy)])

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [101/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [101/80]

end
end
shared_examples_for "strategy with strategy: :build" do |factory_girl_strategy_name|
shared_examples_for "strategy with strategy: :build" do |factory_bot_strategy_name|

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [83/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [83/80]

association_named(:author, factory_girl_strategy_name, great: "value")
expect(factory).to have_received(:run).with(factory_girl_strategy_name, great: "value")
association_named(:author, factory_bot_strategy_name, great: "value")
expect(factory).to have_received(:run).with(factory_bot_strategy_name, great: "value")

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [90/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [90/80]

@@ -18,53 +18,53 @@ def association_named(name, overrides)
end
end
shared_examples_for "strategy with association support" do |factory_girl_strategy_name|
shared_examples_for "strategy with association support" do |factory_bot_strategy_name|

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [86/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [86/80]

@@ -37,7 +37,7 @@
end
describe "a sequence with custom value and aliases" do
subject { FactoryGirl::Sequence.new(:test, 3, aliases: [:alias, :other]) { |n| "=#{n}" } }
subject { FactoryBot::Sequence.new(:test, 3, aliases: [:alias, :other]) { |n| "=#{n}" } }

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Use %i or %I for an array of symbols.
Line is too long. [96/80]

@houndci-bot

houndci-bot Oct 20, 2017

Use %i or %I for an array of symbols.
Line is too long. [96/80]

@@ -26,7 +26,7 @@
end
describe "a sequence with aliases using default value" do
subject { FactoryGirl::Sequence.new(:test, aliases: [:alias, :other]) { |n| "=#{n}" } }
subject { FactoryBot::Sequence.new(:test, aliases: [:alias, :other]) { |n| "=#{n}" } }

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Use %i or %I for an array of symbols.
Line is too long. [94/80]

@houndci-bot

houndci-bot Oct 20, 2017

Use %i or %I for an array of symbols.
Line is too long. [94/80]

end
it "calls the block and returns the result" do
block_run = nil
block = ->(result) { block_run = "changed" }
subject.run(FactoryGirl::Strategy::Build, { }, &block)
subject.run(FactoryBot::Strategy::Build, { }, &block)

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Space inside empty hash literal braces detected.

@houndci-bot

houndci-bot Oct 20, 2017

Space inside empty hash literal braces detected.

describe FactoryBot::Factory, "running a factory" do
subject { FactoryBot::Factory.new(:user) }
let(:attribute) { FactoryBot::Attribute::Static.new(:name, "value", false) }
let(:declaration) { FactoryBot::Declaration::Static.new(:name, "value", false) }

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [85/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [85/80]

let(:declaration) { FactoryGirl::Declaration::Static.new(:name, "value", false) }
describe FactoryBot::Factory, "running a factory" do
subject { FactoryBot::Factory.new(:user) }
let(:attribute) { FactoryBot::Attribute::Static.new(:name, "value", false) }

This comment has been minimized.

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [83/80]

@houndci-bot

houndci-bot Oct 20, 2017

Line is too long. [83/80]

@mike-burns

This comment has been minimized.

Show comment
Hide comment
@mike-burns

mike-burns Oct 20, 2017

Member

I don't know how to review this. Can I safely assume that you programmatically renamed everything?

It looks like you have too many gemspecs.

Member

mike-burns commented Oct 20, 2017

I don't know how to review this. Can I safely assume that you programmatically renamed everything?

It looks like you have too many gemspecs.

@joshuaclayton

This comment has been minimized.

Show comment
Hide comment
@joshuaclayton

joshuaclayton Oct 20, 2017

Member

@mike-burns you're correct; for all non-NEWS references, we updated the name. Additionally, there's a new gemspec file for factory_bot so we can create a new *.gem file.

As for the abundance of lock files, that's due to Appraisal.

Member

joshuaclayton commented Oct 20, 2017

@mike-burns you're correct; for all non-NEWS references, we updated the name. Additionally, there's a new gemspec file for factory_bot so we can create a new *.gem file.

As for the abundance of lock files, that's due to Appraisal.

@Euraldius Euraldius merged commit c716ce0 into master Oct 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound 234 violations found.

@Euraldius Euraldius deleted the aw-rename-to-factory-bot branch Oct 20, 2017

kalys added a commit to kalys/ffactory_girl that referenced this pull request Oct 22, 2017

@pablobm

Should we release a version 5.0 for both Girl and Bot? Although we are not adhering to semantic versioning (are we?), this still is a big change that breaks compatibility, so it may be worth signalling it with a major version change.

If you want to use factory_bot with Rails, see
[factory_bot_rails](https://github.com/thoughtbot/factory_bot_rails).
**A historical note:** factory_bot used to be named factory_girl. An explanation of the name change can be found in the [old factory_girl repository](https://github.com/thoughtbot/factory_girl).
_[Interested in the project name?](NAME.md)._

This comment has been minimized.

@pablobm

pablobm Oct 22, 2017

Member

I think this line (and the associated file) needs to be removed. Maybe replace with an explanation of the name change, which would also tackle my previous comment?

@pablobm

pablobm Oct 22, 2017

Member

I think this line (and the associated file) needs to be removed. Maybe replace with an explanation of the name change, which would also tackle my previous comment?

This comment has been minimized.

@mustmodify

mustmodify Aug 14, 2018

That link now redirects from factory_girl to factory_bot... I'd be interested in knowing why it was renamed. I'm hoping it is something other than "girls shouldn't work in factories."

@mustmodify

mustmodify Aug 14, 2018

That link now redirects from factory_girl to factory_bot... I'd be interested in knowing why it was renamed. I'm hoping it is something other than "girls shouldn't work in factories."

This comment has been minimized.

@mike-burns

mike-burns Aug 14, 2018

Member

Might be nice to have a link to https://robots.thoughtbot.com/factory_bot in here somewhere.

@mike-burns

mike-burns Aug 14, 2018

Member

Might be nice to have a link to https://robots.thoughtbot.com/factory_bot in here somewhere.

If you want to use factory_bot with Rails, see
[factory_bot_rails](https://github.com/thoughtbot/factory_bot_rails).
**A historical note:** factory_bot used to be named factory_girl. An explanation of the name change can be found in the [old factory_girl repository](https://github.com/thoughtbot/factory_girl).

This comment has been minimized.

@pablobm

pablobm Oct 22, 2017

Member

The old respository links to the new one, so this ends up being a link to the current page.

@pablobm

pablobm Oct 22, 2017

Member

The old respository links to the new one, so this ends up being a link to the current page.

3.6+ supports JRuby 1.6.7.2+ while running in 1.9 mode. See [GETTING_STARTED]
for more information on configuring the JRuby environment.
For versions of Ruby prior to 1.9, please use factory_girl 2.x.
For versions of Ruby prior to 1.9, please use factory_bot 2.x.

This comment has been minimized.

@bquorning

bquorning Oct 24, 2017

All changes to this “Supported Ruby versions” section are a bit confusing, since there is currently only a 4.x release of factory_bot.

@bquorning

bquorning Oct 24, 2017

All changes to this “Supported Ruby versions” section are a bit confusing, since there is currently only a 4.x release of factory_bot.

j3pic added a commit to instacart/factory_bot that referenced this pull request Apr 19, 2018

Revert "Replace 'girl' with 'bot' everywhere (thoughtbot#1051)"
This reverts commit c716ce0.

I encountered the deprecation warning in one of our apps, and
realized we'd have to change ALL of the company's apps to
use FactoryBot instead of FactoryGirl. The two can't co-exist.
@mustmodify

This comment was marked as off-topic.

Show comment
Hide comment
@mustmodify

mustmodify Aug 14, 2018

I understand this decision has already been made and it's your code, do what you want. But just to play devil's advocate for a second, if Elon Musk is right and true general AI becomes a thing, might this not make machines feel uncomfortable? 'Bot' could easily be seen as pejorative.

mustmodify commented Aug 14, 2018

I understand this decision has already been made and it's your code, do what you want. But just to play devil's advocate for a second, if Elon Musk is right and true general AI becomes a thing, might this not make machines feel uncomfortable? 'Bot' could easily be seen as pejorative.

@thoughtbot thoughtbot locked as resolved and limited conversation to collaborators Aug 14, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.