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

[Documentation] Fixtures bundle #5286

Merged
merged 7 commits into from
Jun 21, 2016

Conversation

pamil
Copy link
Contributor

@pamil pamil commented Jun 17, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related tickets bundle introduced in #5219, core fixtures in #5285
License MIT

@pamil pamil changed the title [Documentation] Fixtures bundle [WIP] [Documentation] Fixtures bundle Jun 17, 2016
@pjedrzejewski pjedrzejewski added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Jun 17, 2016
options: ~ # Fixture options

They implement ``Sylius\Bundle\FixturesBundle\Fixture\FixtureInterface`` and needs to be registered under
``sylius_fixtures.fixture`` tag in order to be used in suite configuration.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be worthy to present some example definition of service? Code of class and declaration in services.xml. It is explained pretty well, but example can be helpful for beginners. Same for the listener.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upcoming in the next sections :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Custom fixture section added.

@pamil pamil force-pushed the fixtures-bundle-documentation branch from 7f43da2 to c1f9afe Compare June 21, 2016 08:49
@pamil pamil force-pushed the fixtures-bundle-documentation branch from c1f9afe to 65adc94 Compare June 21, 2016 08:53

.. code-block:: yaml

<service id="app.fixture.country" class="AppBundle\Fixture\Country">
Copy link
Contributor

Choose a reason for hiding this comment

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

CountryFixture

@pamil pamil force-pushed the fixtures-bundle-documentation branch from 65adc94 to 5c59930 Compare June 21, 2016 09:32
@pamil pamil force-pushed the fixtures-bundle-documentation branch from 5c59930 to a4b71ec Compare June 21, 2016 09:54
@pamil pamil changed the title [WIP] [Documentation] Fixtures bundle [Documentation] Fixtures bundle Jun 21, 2016
@pamil pamil force-pushed the fixtures-bundle-documentation branch from a4b71ec to 981b351 Compare June 21, 2016 09:58
priority: 0 # The higher priority is, the sooner the fixture will be executed
options: ~ # Fixture options

They implement ``Sylius\Bundle\FixturesBundle\Fixture\FixtureInterface`` and needs to be registered under
Copy link
Member

Choose a reason for hiding this comment

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

... the Sylius\Bundle\FixturesBundle\Fixture\FixtureInterface and need to be registered under the sylius_fixtures.fixture tag ...


$this->countryManager->flush();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add docblocks in the examples? : D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, it will make listings unnecessarily longer and won't make anything clearer as those methods receives only typehinted parameters and usually does not return anything (except getName() which is quite straightforward).

@CoderMaggie
Copy link
Member

CoderMaggie commented Jun 21, 2016

Only some corrections of articles, plus docblocks in code examples - for me looks really fine, clear and understandable! :) 👍

@pamil
Copy link
Contributor Author

pamil commented Jun 21, 2016

Thank you for so many great suggestions @TheMadeleine! 🎉

@pjedrzejewski pjedrzejewski merged commit 36d1ea7 into Sylius:master Jun 21, 2016
@pjedrzejewski
Copy link
Member

Nice! Thank you. 👍

@pjedrzejewski
Copy link
Member

I agree about skipping docblocks and not sure about <?php tag, if Symfony skips it, I think we can safely do it too!

@pamil pamil deleted the fixtures-bundle-documentation branch June 21, 2016 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants