Skip to content

Reorder DoctrineBundle configuration sections #8573

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

Merged

Conversation

marcverney
Copy link
Contributor

I think the DoctrineBundle Configuration page is not structured correctly. Here are the current page's sections, along with my comments:

  1. Full Default Configuration

    Here the dbal conf is presented before the orm conf (which is perfectly ok). The reader will therefore expect that in the following sections, the dbal conf is presented first. This is not the case.

  2. Configuration Overview

    Actually this is orm-only. The section title should be explicit about this.

    2.1. Caching Drivers

    The YAML example uses the shortened configuration syntax, which has not been presented yet (it's in section 4).

    2.2. Mapping Configuration

  3. Doctrine DBAL Configuration

    This section should come before the previous one, as per my comment in section 1.

  4. Shortened Configuration Syntax

    This is orm-related, therefore should be a subsection of 2.

  5. Custom Mapping Entities in a Bundle

    This is orm related, therefore should be a subsection of 2

  6. Mapping Entities Outside of a Bundle

    This is orm related, therefore should be a subsection of 2

This PR reorders the sections this way:

  1. Full Default Configuration
  2. Doctrine DBAL Configuration
  3. Doctrine ORM Configuration (formerly "Configuration Overview")
    2.1. Shortened Configuration Syntax
    2.2. Caching Drivers
    2.3. Mapping Configuration
    2.4. Custom Mapping Entities in a Bundle
    2.5. Mapping Entities Outside of a Bundle

@weaverryan
Copy link
Member

Wow, this is great! Yea, it's a mess. I don't think anyone ever really thought about the article holistically. So yes, let's fix it! I have a few other thoughts:

  1. See Simplified the framework configuration reference #7201. It's outside of the scope of this PR a bit... but honestly, I would love to mention the commands (config:dump & debug:config) to the top of every reference article.

  2. I'm thinking more about the "Full Default Configuration". I'm trying to decide if it is an "annoying" big code block that should be moved to the bottom, or if it is the BEST thing ever, and we should try to include as much documentation in here (as comments) and minimize the amount of docs that are needed after this. I'm leaning towards the second: keep the "Full Default Configuration" on top, and, going forward, try to make it really great, by adding comments and even examples. @javiereguiluz what do you think about this

  3. There is a conflict on this PR somehow - can you check into that? :)

Thanks!

@marcverney
Copy link
Contributor Author

Fixed the conflict. As for the other two points you mention, I'm not sure they must be adressed in this PR (if this is what you suggested). I'm +1 for "keep the Full Default Configuration on top, and [...] try to make it really great", FWIW.

@weaverryan
Copy link
Member

Thanks Marc! I hope we'll see more PR's from you ;)

@weaverryan weaverryan merged commit 79806ac into symfony:2.7 Nov 5, 2017
weaverryan added a commit that referenced this pull request Nov 5, 2017
…y, marcverney)

This PR was merged into the 2.7 branch.

Discussion
----------

Reorder DoctrineBundle configuration sections

I think the DoctrineBundle Configuration page is not structured correctly. Here are the current page's sections, along with my comments:

1. [Full Default Configuration](https://symfony.com/doc/master/reference/configuration/doctrine.html#full-default-configuration)

    Here the `dbal` conf is presented before the `orm` conf (which is perfectly ok). The reader will therefore expect that in the following sections, the `dbal` conf is presented first. This is not the case.

1. [Configuration Overview](https://symfony.com/doc/master/reference/configuration/doctrine.html#configuration-overview)

    Actually this is orm-only. The section title should be explicit about this.

    2.1. [Caching Drivers](https://symfony.com/doc/master/reference/configuration/doctrine.html#caching-drivers)

    The YAML example uses the shortened configuration syntax, which has not been presented yet (it's in section 4).

    2.2. [Mapping Configuration](https://symfony.com/doc/master/reference/configuration/doctrine.html#mapping-configuration)

1. [Doctrine DBAL Configuration](https://symfony.com/doc/master/reference/configuration/doctrine.html#doctrine-dbal-configuration)

    This section should come before the previous one, as per my comment in section 1.

1. [Shortened Configuration Syntax](https://symfony.com/doc/master/reference/configuration/doctrine.html#shortened-configuration-syntax)

    This is orm-related, therefore should be a subsection of 2.

1. [Custom Mapping Entities in a Bundle](https://symfony.com/doc/master/reference/configuration/doctrine.html#custom-mapping-entities-in-a-bundle)

    This is orm related, therefore should be a subsection of 2

6. [Mapping Entities Outside of a Bundle](https://symfony.com/doc/master/reference/configuration/doctrine.html#mapping-entities-outside-of-a-bundle)

    This is orm related, therefore should be a subsection of 2

This PR reorders the sections this way:

1. Full Default Configuration
3. Doctrine DBAL Configuration
2. Doctrine ORM Configuration (formerly "Configuration Overview")
    2.1. Shortened Configuration Syntax
    2.2. Caching Drivers
    2.3. Mapping Configuration
    2.4. Custom Mapping Entities in a Bundle
    2.5. Mapping Entities Outside of a Bundle

Commits
-------

79806ac Merge branch '2.7' into reorder-doctrine-conf-ref-sections
996b219 Reorder DoctrineBundle configuration sections
@marcverney marcverney deleted the reorder-doctrine-conf-ref-sections branch November 6, 2017 14:51
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.

3 participants