-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Update data_transformers.rst #5307
Conversation
zebba
commented
May 23, 2015
Q | A |
---|---|
Doc fix? | yes |
New docs? | no |
Applies to | 2.3+ |
Fixed tickets | #2471 |
Minor change.
Minor change.
Now that you have the transformer built, you just need to add it to your | ||
issue field in some form. | ||
As seen above our transformer requires an instance of an object manager. While for most | ||
use-cases using the default manager is fine we will let you pick the manager by it's name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[...] is fine, we will [...]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We avoid the first person perspective in the docs. So we should reword the sentence a bit like the following:
While for most use-cases it is sufficient to use the default entity manager, you will sometimes need to explicitly choose the one to use. To achieve this, you can use a factory::
// src/Acme/TaskBundle/Form/DataTransformer/IssueToNumberTransformerFactory.php | ||
namespace Acme\TaskBundle\Form\DataTransformer; | ||
|
||
use Symfony\Bridge\Doctrine\ManagerRegistry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have any reason why you don't use Doctrine\Common\Persistence\ManagerRegistry
, but require the specific bridge implementation?
Couple of changes.
All suggestions have been implemented, so this PR seems ready to be merged ... or at least to label it as Finished. Thanks. |
This PR was merged into the 2.3 branch. Discussion ---------- Update data_transformers.rst | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | #2471 Commits ------- 939cfc5 Update data_transformers.rst 4b09b3f Update data_transformers.rst 7929ed5 Update data_transformers.rst 19324e8 Update data_transformers.rst
…averryan) This PR was merged into the 2.3 branch. Discussion ---------- Completely re-reading the data transformers chapter | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | #5307 **Note**: `setDefaultOptions()` -> `configureOptions()` in 2.7 after merge Hi guys! This follows after #5307, which helped update the article (but also added some new complexity). Data transformers are super cool, but I think we've over-complicated them in the past. Even for me, before re-writing this, I tried to avoid transformers because I thought they were hard :). The code for this can be found here: https://github.com/weaverryan/docs-data-transformer/commits/finish, where each commit that starts with "[tuts-apply-diff]" is a "step" in the tutorial. My goals: 1) Show an easy example using the CallbackTransformer in the beginning 2) Remove complexity related to any factories that were there before 3) Moved theoretical section to the bottom Thanks! Commits ------- 0762848 Adding an example of how the other format for attaching transformers looks 28d7f89 fixed typo thanks to @OskarStark 41bb907 adding use statement 1514fdd using the aliases entity manager service name 9be07f0 Many tweaks thanks to review b48feda Completely re-reading the data transformers chapter: