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

add informations how to create a custom doctrine mapping #4700

Closed
wants to merge 2 commits into from

Conversation

timglabisch
Copy link
Contributor

i tryed to explain a bit how doctrine tries to find the correct mapping, the driver and how you can configure your own mapping.

@@ -485,4 +485,68 @@ can be placed directly under ``doctrine.orm`` config level.
This shortened version is commonly used in other documentation sections.
Keep in mind that you can't use both syntaxes at the same time.


Custom Mapping
---------------
Copy link
Member

Choose a reason for hiding this comment

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

the above 2 lines should have exact the same length

@wouterj
Copy link
Member

wouterj commented Dec 29, 2014

The config references surely needs some love, thanks for starting this!

@timglabisch
Copy link
Contributor Author

@wouterj thanks for reviewing this, hopefully i fixed everything :)

),
));


Copy link
Member

Choose a reason for hiding this comment

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

remove one blank line here

@xabbuh
Copy link
Member

xabbuh commented Dec 30, 2014

@timglabisch You should nest the different code blocks into a configuration-block directive.

Detecting a Mapping A MetadataDriver (type)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If the ``type`` on the bundle configuration isn't set, the DoctrineBundle will try to detect the correct metadata driver
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 please wrap lines after the first word that crosses the 72nd character?

@timglabisch
Copy link
Contributor Author

please rereview.

on your own.

For example, you can overwrite the configuration directory for a given
bundle. Keep in mind that the bundle you want to overwrite has to exist.
Copy link
Member

Choose a reason for hiding this comment

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

I would combine these 2 paragraphs into one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh asks me to wrap this at the 72nd char. should i move the bundle one line up?

@timglabisch
Copy link
Contributor Author

@wouterj @xabbuh can you take a look at this PR, whats missing?

auto_mapping: true
mappings:
# ...
TgDoctrineMappingBundle:
Copy link
Member

Choose a reason for hiding this comment

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

AppBundle

@timglabisch
Copy link
Contributor Author

@xabbuh i changed the namespace, please review

@weaverryan
Copy link
Member

Hi Tim!

Thanks for this! I just left a lot of comments, but it's more because this configuration is confusing, with all the is_bundle stuff and how behaviors change (which I'm guessing, is why you decided to document it). Anyways, for me, it wasn't quite clear yet - so I added some suggestions. Please make any changes that make sense and ask any questions.

Cheers!

@timglabisch
Copy link
Contributor Author

@weaverryan the doctrine configuration here is a bit weird, can you please rereview this? i did some additional changes to make this more clear.

Custom Mapping Entities in a Bundle
-----------------------------------

Doctrine's ```auto_mapping`` feature loads annotation configuration from the
Copy link
Member

Choose a reason for hiding this comment

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

extra tick here

@weaverryan
Copy link
Member

Hey Tim!

Thanks for the awesome-fast turn-around. I just added some more notes. Ultimately, we may need to re-make this PR against DoctrineBundle, as we've decided to keep this configuration file in their repository. Let's perfect this first, then re-make it over there.

Thanks!

@timglabisch
Copy link
Contributor Author

thanks @weaverryan please rereview this PR.

@timglabisch
Copy link
Contributor Author

ping @wouterj @xabbuh

),
));

Mapping Entities outside of a Bundle
Copy link
Member

Choose a reason for hiding this comment

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

minor: "outside" should be capitialized

@timglabisch
Copy link
Contributor Author

@wouterj fixed

@weaverryan
Copy link
Member

This looks great to me. Now, we really need this to go into the DoctrineBundle itself: https://github.com/doctrine/DoctrineBundle/blob/master/Resources/doc/configuration.rst. @timglabisch can you duplicate this PR over there?

We also need finish "moving" the configuration section to that repository. It looks like our reference section may still have some stuff that their's doesn't. We need to make sure everything we have is in their configuration section, then replace our reference section with a link to their docs.

Thanks!

@wouterj
Copy link
Member

wouterj commented May 21, 2015

@weaverryan shouldn't we just merge this one?

@weaverryan
Copy link
Member

Yep - I've merged this in - the moving to Doctrine should be a separate issue. Sorry about the delay and thanks for the re-ping Wouter!

weaverryan added a commit that referenced this pull request May 22, 2015
…g (timglabisch)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #4700).

Discussion
----------

add informations how to create a custom doctrine mapping

i tryed to explain a bit how doctrine tries to find the correct mapping, the driver and how you can configure your own mapping.

Commits
-------

8a98fc1 Update doctrine.rst
bec3c98 Doctrine Custom Mapping
@weaverryan weaverryan closed this May 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants