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

Improve documentation about naming strategy usage #8041

Merged
merged 1 commit into from Mar 1, 2020

Conversation

RosemaryOrchard
Copy link
Contributor

Add a note/warning that annotations override the naming strategy.

Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, here are some thoughts on how to make it better :)

Comment on lines 24 to 29



Warning

The naming strategy is always overridden by annotations on an entity.
Copy link
Member

@lcobucci lcobucci Feb 26, 2020

Choose a reason for hiding this comment

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

To render a nice warning block we should format this like:

.. warning

    The naming strategy is always overridden by annotations on an entity.

I'd also suggest moving this block to the intro (right before the titleConfiguring a naming strategy) to make things a bit more explicit.
Text-wise I think we could a bit more verbose since I noticed that we do mention this on the intro "when the column or table name is not given".

Perhaps removing the "when the column or table name is not given" and saying something like "Naming strategies are completely ignored when table or column names are provided in the entity mapping", what do you think?

@lcobucci lcobucci added this to the 2.7.2 milestone Feb 26, 2020
@lcobucci lcobucci changed the title Annotations override naming strategy Improve documentation about naming strategy usage Feb 26, 2020
@RosemaryOrchard
Copy link
Contributor Author

I've moved it up, changed the formatting and removed the part of the intro that implied this but didn't explicitly state it.

Add a note/warning that annotations override the naming strategy.
Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

Thank you

@beberlei beberlei merged commit 9273057 into doctrine:2.7 Mar 1, 2020
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.

None yet

5 participants