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.5] Make seeders chainable #22288

Merged
merged 1 commit into from Dec 3, 2017

Conversation

Projects
None yet
9 participants
@freekmurze
Contributor

freekmurze commented Dec 2, 2017

Right now you must call seeders in DatabaseSeeder like this:

$this->call(SeederA::class);
$this->call(SeederB::class);
$this->call(SeederC::class);

The changes in this PR will enable to do the above like this

$this
   ->call(SeederA::class);
   ->call(SeederB::class);
   ->call(SeederC::class);

@freekmurze freekmurze changed the title from Make seeders chainable to [5.5] Make seeders chainable Dec 2, 2017

@antonkomarev

This comment has been minimized.

Contributor

antonkomarev commented Dec 2, 2017

What is the profit from it?

Wouldn't it be better to make:

$this->call([
    SeederA::class,
    SeederB::class,
    SeederC::class,
]);

Or:

$this->calls([
    SeederA::class,
    SeederB::class,
    SeederC::class,
]);
@devcircus

This comment has been minimized.

Contributor

devcircus commented Dec 3, 2017

Can't you already do

$this->call([
    SeederA::class,
    SeederB::class,
]);

@taylorotwell taylorotwell merged commit a919819 into laravel:5.5 Dec 3, 2017

2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@browner12

This comment has been minimized.

Contributor

browner12 commented Dec 4, 2017

@devcircus yes you can. I'm not really seeing the benefit of chaining, but oh well, it's already in.

@freekmurze freekmurze deleted the freekmurze:chainable-seeders branch Dec 4, 2017

@lbm-trentm

This comment has been minimized.

lbm-trentm commented Dec 9, 2017

if no one can really see the benefit to this, why bloat the software?

@nachofassini

This comment has been minimized.

nachofassini commented Dec 11, 2017

what @a-komarev proposes its already implemented. Seeder docs

@deleugpn

This comment has been minimized.

Contributor

deleugpn commented Dec 11, 2017

@lbm-trentm The PR creator, people liking the PR and Taylor seem to disagree with your premise.

@antonkomarev

This comment has been minimized.

Contributor

antonkomarev commented Dec 11, 2017

@deleugpn nobody said that we are disagree with this PR. Question of reason was raised and this behavior was available already, as @nachofassini pointed to docs.

One minor notice that in first comment bad example is provided. It should be:

$this
   ->call(SeederA::class)
   ->call(SeederB::class)
   ->call(SeederC::class);
@connectkushal

This comment has been minimized.

Contributor

connectkushal commented Dec 11, 2017

to those grumbling about bloating the software without any benefit, this pr adds just 2 extra words to the actual code, return $this 🙄

@browner12

This comment has been minimized.

Contributor

browner12 commented Dec 11, 2017

Lingchi - Death by 1000 cuts.

Mostly trolling, but our initial point was this was a solution to a problem that already had a solution.

It's in now, so /shrug

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