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

DQL custom functions on doctrine reference page #3946

Closed
wants to merge 11 commits into from

Conversation

healdropper
Copy link
Contributor

Custom DQL functions can be configured in two ways, according to what I was told here

It should be clearified in doctrine reference page because It may confuse some people that just read the cookbook section about custom DQL functions

Seems that there is two possibilities to configure custom DQL functions

It should be clearified in doctrine reference page because It may confuse some people if they just read the cookbook: http://symfony.com/doc/current/cookbook/doctrine/custom_dql_functions.html
@wouterj
Copy link
Member

wouterj commented Jun 13, 2014

This applies to all settings for entity managers, can you please make it a bit more generic.

Btw, I love your work :) It's just a minor thing

@healdropper
Copy link
Contributor Author

Sure, as soon as I am back at home. Thanks!

Changed documentation to explain shorten syntax in general, not just DQL functions part
@healdropper
Copy link
Contributor Author

I think it's quite clear now, let me know your opinion about it. Thanks!

orm:
# ...
query_cache_driver:
type: array # Required
Copy link
Member

Choose a reason for hiding this comment

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

I think - to avoid duplication (but to keep things clear) we should only show the top-level items (e.g. query_cache_driver, metadata_cache_driver, etc) and put a #... under each if it has child options. So:

query_cache_driver:
    # ...
metadata_cache_driver:
    # ...
result_cache_driver:
    # ...
connection: ~
class_metadata_factory_name: Doctrine\ORM\Mapping\ClassMetadataFactory

and then continued for everything else. What do you think about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably it will be easier to understand, yes. The shorter, the better in this case.

Made more clear the shorten doctrine.yml syntax, leaving  only the first level of configuration.
@@ -411,3 +411,37 @@ Each connection is also accessible via the ``doctrine.dbal.[name]_connection``
service where ``[name]`` is the name of the connection.

.. _DBAL documentation: http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html

Shorten configuration syntax
Copy link
Member

Choose a reason for hiding this comment

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

Shorten Configuration Syntax

Proposed changes to format
filters:
# ...

This shorten version is commonly used in other documentation sections.
Copy link
Member

Choose a reason for hiding this comment

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

shortened

@weaverryan
Copy link
Member

@healdropper There are just a few very minor things left! If you can make those updates, I'll merge!

Minor typos changes
More typos oops :P
Fix for "Title underline too short"
@healdropper
Copy link
Contributor Author

@weaverryan There you go! Thanks!

@@ -411,3 +411,39 @@ Each connection is also accessible via the ``doctrine.dbal.[name]_connection``
service where ``[name]`` is the name of the connection.

.. _DBAL documentation: http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html

Shortened Configuration syntax
Copy link
Member

Choose a reason for hiding this comment

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

Syntax should use an uppercased S

Included more suggestions
@weaverryan
Copy link
Member

Thanks @healdropper for your hard work. I think this is a great new note! Cheers!

@weaverryan weaverryan closed this in 4a9e49e Jul 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants