Skip to content

Error on Cookbook #4032

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

Closed
ajouve opened this issue Jul 17, 2014 · 8 comments
Closed

Error on Cookbook #4032

ajouve opened this issue Jul 17, 2014 · 8 comments
Labels
actionable Clear and specific issues ready for anyone to take them. Form good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.

Comments

@ajouve
Copy link

ajouve commented Jul 17, 2014

I don't know if this is really an error, on this page:
https://github.com/symfony/symfony-docs/blob/master/cookbook/form/data_transformers.rst

or

http://symfony.com/doc/current/cookbook/form/data_transformers.html

I have an error when I am using $options['em'], the key does not exists

@stof
Copy link
Member

stof commented Jul 17, 2014

There is no error. The form field defined in the doc puts em as a required option, so it exist in buildForm

@javiereguiluz
Copy link
Member

@stof it's not an error in the code, but I think that @ajouve is right about being something that we should improve. Right now, this is the only information that readers can get from the code:

public function buildForm(FormBuilderInterface $builder, array $options)
{
    // ...

    // this assumes that the entity manager was passed in as an option
    $entityManager = $options['em'];

"This assumes that the EM was passed ..." I don't think it's clear enough who passed it, where and how.

@weaverryan
Copy link
Member

What do you think about changing the comment to this:

// the "em" is an option that you pass when creating your form. Check out
// the 3rd argument to createForm in the next code block to see how this
// is passed to the form (also see setDefaultOptions).

The usage example (createForm with the em option being passed), is shown on this page. I think if you actually see it, it would clear things up.

@javiereguiluz
Copy link
Member

@weaverryan I like your proposal. The new comment is definitely much better! Thank you.

@javiereguiluz
Copy link
Member

@raulfraile I see your commit related to this minor change (raulfraile@b241206) but I don't know if you finally managed to create a pull request to the symfony-docs repo. If not, can we do something to help you? Thanks!

@javiereguiluz
Copy link
Member

This issue seems to be fixed in raulfraile/symfony-docs@b241206 but it was never merged. Can we do anything about this?

@xabbuh
Copy link
Member

xabbuh commented Feb 18, 2015

ping @raulfraile

raulfraile added a commit to raulfraile/symfony-docs that referenced this issue Feb 18, 2015
@raulfraile
Copy link
Contributor

@javiereguiluz @xabbuh I just created a PR for this, I hope it's ok!

@wouterj wouterj added the hasPR A Pull Request has already been submitted for this issue. label Feb 19, 2015
weaverryan added a commit that referenced this issue Mar 14, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

#4032 improved comments about em option

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #4032

Commits
-------

c5b8926 #4032 improved comments about em option
weaverryan added a commit that referenced this issue Mar 14, 2015
* 2.3:
  [#5015] Very small tweak
  [#5011] Adding one more fix I missed
  [#5011] Fixing minor build issue
  improved link
  fixed intendation and shortened the link
  Adding a break statement to improve the sample code
  Added an example about how to get the impersonating user object
  #4032 improved comments about em option
  tip for mapping definition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Clear and specific issues ready for anyone to take them. Form good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Development

No branches or pull requests

7 participants