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

added faker fixture command #1527

Closed
wants to merge 6 commits into from
Closed

added faker fixture command #1527

wants to merge 6 commits into from

Conversation

@Ragazzo
Copy link
Contributor

@Ragazzo Ragazzo commented Dec 14, 2013

Done, this is for issue #1506
Things to discuss:

  • i think we need basic fixture command that will have actionUp/actionDown to apply fixtures to the table or reset table, very simple to implement. Also this will help faker\FixtureController to be more useful for users because of faker\FixtureController < console\FixtureController
  • Do we need README.md in the extension repo?

@samdark @qiangxue maybe some other thoughts/suggestions? other developers? :)

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 14, 2013

Example:

# users.php template
return [
    'name'  =>  'name',
    'phone' =>  'phoneNumber',
    'city'  =>  'city',
];

English version:

#users.php generated fixture

return [
    [
        'name' => "Litzy Effertz Sr.",
        'phone' => "300-413-7407",
        'city' => "East Eraport",
    ],
    [
        'name' => "Marcelina Shanahan",
        'phone' => "(699)210-2743",
        'city' => "Boscostad",
    ],
    [
        'name' => "Mrs. Nico Reynolds IV",
        'phone' => "1-716-733-0810x033",
        'city' => "South Clemmieberg",
    ],
    [
        'name' => "Mary Wolf",
        'phone' => "464-410-2653",
        'city' => "West Heath",
    ],
    [
        'name' => "Kyla Gottlieb",
        'phone' => "(479)691-1691x63812",
        'city' => "Port Dashawn",
    ],
];

Russian version:

return [
    [
        'name' => "Ситников Витольд Владимирович",
        'phone' => "(35222) 97-2086",
        'city' => "Дмитров",
    ],
    [
        'name' => "Ковалёв Захар Владимирович",
        'phone' => "+7 (922) 574-6047",
        'city' => "Клин",
    ],
    [
        'name' => "Романов Анатолий Алексеевич",
        'phone' => "(495) 980-8701",
        'city' => "Видное",
    ],
    [
        'name' => "Мишин Валериан Владимирович",
        'phone' => "(495) 637-3634",
        'city' => "Дорохово",
    ],
    [
        'name' => "Носов Прохор Максимович",
        'phone' => "+7 (922) 117-4446",
        'city' => "Дорохово",
    ],
];

there are a lot of languages and formatters (data providers) see this guide https://github.com/fzaninotto/Faker and source https://github.com/fzaninotto/Faker/tree/master/src/Faker/Provider

@cebe
Copy link
Member

@cebe cebe commented Dec 14, 2013

Do we need README.md in the extension repo?

Yes and also composer.json + license + changelog. you can copy them from other extensions.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 14, 2013

@cebe done, thanks.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 15, 2013

added base fixture controller. i think its ready for review and maybe merge :)

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 15, 2013

also added default actions so this commands will be more useful.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 17, 2013

i cloned new my Yii2 repo so this branch is not available for me anymore. If there will be some bugs/suggestions they can be fixed/added after this one merge. Anyway this one is ready for review.

@cebe
Copy link
Member

@cebe cebe commented Dec 17, 2013

i cloned new my Yii2 repo so this branch is not available for me anymore.

assuming your repo remote is origin, just do:

git fetch origin
git checkout yii2-faker-integration

and you'll have it back.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 17, 2013

thanks will do, i thought branch was automatically deleted :(

@Ragazzo Ragazzo mentioned this pull request Dec 17, 2013
5 of 5 tasks complete
*
* class Book extends \Faker\Provider\Base
* {
* public function title($nbWords = 5)

This comment has been minimized.

@samdark

samdark Dec 25, 2013
Member

What does nb mean in $nbWords? Can it be renamed to be meaningful?

This comment has been minimized.

@Ragazzo

Ragazzo Dec 25, 2013
Author Contributor

its just an example taken from Faker official guide. This is not part of current PR.

This comment has been minimized.

@samdark

samdark Dec 25, 2013
Member

It is in PR diff. That means it is part of it, isn't?

This comment has been minimized.

@Ragazzo

Ragazzo Dec 25, 2013
Author Contributor

yes, but it is in docs as you can see. Just an example for users so they dont need to go to Faker guide and search it there. As you can see it is very big https://github.com/fzaninotto/Faker and this example is just in the half of it.

* public function title($nbWords = 5)
* {
* $sentence = $this->generator->sentence($nbWords);
* return substr($sentence, 0, strlen($sentence) - 1);

This comment has been minimized.

@samdark

samdark Dec 25, 2013
Member

If sentence is non-English then you need mb_* in order for it to work correctly.

This comment has been minimized.

@Ragazzo

Ragazzo Dec 25, 2013
Author Contributor

its just an example! :D we can remove it at all. Ofcourse i will use stringhelper in Yii, but it is just an example :)

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Examples should be as correct as possible. It is what people will reuse as is a lot.

*
* ~~~
* 'controllerMap' => [
* 'faker:fixture' => [

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Alignment is ugly here.

public $templatePath = '@tests/unit/fixtures/templates';

/**
* Language to use when generating fixtures data.

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Should be @var string Language to use when generating fixtures data. as per phpdoc standard. Also it worth mentioning what is default value.

/**
* Additional data providers that can be created by user and will be added to the Faker generator.
* More info in Faker library docs https://github.com/fzaninotto/Faker.
* @var array

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Wrong phpdoc formatting, see above. Also More info is typically added as @link.

* More info in Faker library docs https://github.com/fzaninotto/Faker.
* @var array
*/
public $providers = array();

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Short array style should be used.


/**
* Faker generator instance
* @var \Faker\Generator

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Wrong phpdoc style, see above.

$template = require($templateFile);
$content = "<?php\n\nreturn [";

for ($i = 0; $i < $times; $i++)

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Formatting should be fixed.

*/
public function needToGenerateAll($file)
{
return $file == 'all_fixtures';

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Is it a good idea to move all_fixtures to class level constant?

Faker Extension for Yii 2
===============================

This extension provides a `Faker` fixture command for Yii 2.

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

What is it for? What should I think if I don't know what "Faker" is?

],
],
```
To start using this command you need to be familiar (read guide) for the Faker library and

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Provide a link to faker guide.


```php
'controllerMap' => [
'faker:fixture' => [

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

It's better to use just faker. Less to type.

@@ -0,0 +1,27 @@
{
"name": "yiisoft/yii2-faker",
"description": "The Faker integration for the Yii framework",

This comment has been minimized.

@samdark

samdark Dec 26, 2013
Member

Worth mentioning that it's fixture generator.

@samdark
Copy link
Member

@samdark samdark commented Dec 26, 2013

Looks OK to me overall. Haven't actually tried using it though.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 26, 2013

ok, will fix all your suggestions.

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 27, 2013

i think i will close this PR, because of i cant get this branch back (it was in my Yii2 github repo, but then i deleted it and forked new one from main Yii2 repo). So i guess i need to manually restore all files.

@Ragazzo Ragazzo closed this Dec 27, 2013
@cebe
Copy link
Member

@cebe cebe commented Dec 30, 2013

you can do

git fetch origin
git checkout yii2-faker-integration

to get your branch back. assuming origin is the name of your forks git remote

@Ragazzo
Copy link
Contributor Author

@Ragazzo Ragazzo commented Dec 30, 2013

@cebe i cant. i re-forked repo on github: deleted and then forked once again. my current remote dont have such branch.

@cebe
Copy link
Member

@cebe cebe commented Dec 30, 2013

oh okay.

@Ragazzo Ragazzo mentioned this pull request Jan 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.