Skip to content

Updated reference articles to Symfony 4 #8643

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
merged 5 commits into from
Nov 27, 2017

Conversation

javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented Nov 13, 2017

This fixes #8642.


I want to update these two files more deeply in a separate PR, so here their changes are minimal:

  • reference/events.rst
  • reference/dic_tags.rst

Now, some questions:

1) I've moved all custom Doctrine clases (custom DQL functions, custom DBAL types, custom database drivers, etc.) under App\Bridge\Doctrine\ namespace (e.g. Acme\HelloBundle\DQL\DatetimeFunction -> App\Bridge\Doctrine\DQL\DatetimeFunction). I saw this in a Symfony app and I like it. Do you agree?

2) I don't know how to update this section from /reference/configuration/framework.rst:

resources
"""""""""

**type**: ``string[]`` **default**: ``['FrameworkBundle:Form']``

A list of all resources for form theming in PHP. This setting is not required
if you're using the Twig format for your templates, in that case refer to
:ref:`the form article <forms-theming-twig>`.

Assume you have custom global form themes in
``src/Resources/views/Form``, you can configure this like:

.. configuration-block::

    .. code-block:: yaml

        # config/packages/framework.yaml
        framework:
            templating:
                form:
                    resources:
                        - 'WebsiteBundle:Form'

    [...]

.. note::

    The default form templates from ``FrameworkBundle:Form`` will always
    be included in the form resources.

.. seealso::

    See :ref:`forms-theming-global` for more information.

3) The reference/configuration/doctrine.rst contains A LOT of references to bundles, including options designed for bundles (prefix, alias, is_bundle) What should we do?

@wouterj wouterj added this to the 4.0 milestone Nov 13, 2017
xabbuh added a commit that referenced this pull request Nov 14, 2017
This PR was squashed before being merged into the 3.3 branch (closes #8644).

Discussion
----------

Removed an uneeded deprecation notice

Spotted while working on #8643.

Commits
-------

a96a633 Removed an uneeded deprecation notice
@weaverryan
Copy link
Member

  1. I actually don't love the Bridge part. It doesn't have much "meaning", and it would be harder to find Doctrine stuff. Do you have a counter-argument?

  2. ... the form theming stuff... I guess that's still supported? So we should keep it for now. I think this is still accurate as-is. Except for:

src/Resources/views/Form -> templates/form_themes
'WebsiteBundle:Form' -> form_themes

But I'm not 100% if that's accurate. But I think we should just try it.

  1. Hmm. Emphasis should definitely be given to the mapping configuration that we use for Flex (e.g. https://github.com/symfony/recipes/blob/db32040b48e9cf15299e7702a75a49e76999ff3f/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml#L9-L15). Should we continue to show is_bundle: true? Hmm, maybe just mention it. But I think we should show is_bundle: false-based configuration.

@weaverryan
Copy link
Member

Status: Needs work

@xabbuh xabbuh changed the base branch from master to 4.0 November 22, 2017 12:22
@javiereguiluz
Copy link
Member Author

I've made all requested changes. Later we can further reword the part about bundles/non-bundles to reduce the bundles section to a note/tip ... but for now this could be good enough. Thanks.

@weaverryan weaverryan merged commit f709035 into symfony:4.0 Nov 27, 2017
weaverryan added a commit that referenced this pull request Nov 27, 2017
This PR was squashed before being merged into the 4.0 branch (closes #8643).

Discussion
----------

Updated reference articles to Symfony 4

This fixes #8642.

-----

I want to update these two files more deeply in a separate PR, so here their changes are minimal:

* `reference/events.rst`
* `reference/dic_tags.rst`

-----

Now, some questions:

**1)** I've moved all custom Doctrine clases (custom DQL functions, custom DBAL types, custom database drivers, etc.) under `App\Bridge\Doctrine\` namespace (e.g. `Acme\HelloBundle\DQL\DatetimeFunction` -> `App\Bridge\Doctrine\DQL\DatetimeFunction`). I saw this in a Symfony app and I like it. Do you agree?

**2)** I don't know how to update this section from `/reference/configuration/framework.rst`:

```rst
resources
"""""""""

**type**: ``string[]`` **default**: ``['FrameworkBundle:Form']``

A list of all resources for form theming in PHP. This setting is not required
if you're using the Twig format for your templates, in that case refer to
:ref:`the form article <forms-theming-twig>`.

Assume you have custom global form themes in
``src/Resources/views/Form``, you can configure this like:

.. configuration-block::

    .. code-block:: yaml

        # config/packages/framework.yaml
        framework:
            templating:
                form:
                    resources:
                        - 'WebsiteBundle:Form'

    [...]

.. note::

    The default form templates from ``FrameworkBundle:Form`` will always
    be included in the form resources.

.. seealso::

    See :ref:`forms-theming-global` for more information.
```

**3)** The `reference/configuration/doctrine.rst` contains A LOT of references to bundles, including options designed for bundles (`prefix`, `alias`, `is_bundle`) What should we do?

* Remove all references to bundles, including those options?
* Maintain all those options but update their explanations using the default Symfony Flex Recipe config? (see https://github.com/symfony/recipes/blob/db32040b48e9cf15299e7702a75a49e76999ff3f/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml#L9-L15)
* ...?

Commits
-------

f709035 Restored the Assetic tags because they are removed in another PR
206bf70 Updated Doctrine config reference about bundles
e80995b Fixed PHP form theme resources
ef2a044 Remove App\Bridge\Doctrine\ namespace
da67105 Updated reference articles to Symfony 4

alias
.....

Doctrine offers a way to alias entity namespaces to simpler, shorter names
to be used in DQL queries or for Repository access. When using a bundle
the alias defaults to the bundle name.
to be used in DQL queries or for Repository access. It's default value is ``App``.
Copy link
Member

Choose a reason for hiding this comment

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

Btw, I took out these "default value is". These aren't really the default values - it's just what you get out-of-the-box with the recipe. And I think if you're configuring these things, then you are clearly looking at your doctrine.yaml file, so you will see those values there I think not mentioning these is a bit less confusing.

weaverryan added a commit that referenced this pull request Nov 27, 2017
These aren't the defaults: they're the values that you get in your
config file from the recipe.
weaverryan added a commit that referenced this pull request Nov 27, 2017
* 4.0: (36 commits)
  Update event_listeners_subscribers.rst
  Update event_listeners_subscribers.rst
  [#8683] Tweaks based on feedback
  [#8643] Removing default values
  [#8734][#8736] Adding missing installation details
  fixes thanks to the team!
  add missing link target for Ansistrano
  Restored the Assetic tags because they are removed in another PR
  Corrected a spelling error.
  Updated Doctrine config reference about bundles
  Fixed PHP form theme resources
  Remove App\Bridge\Doctrine\ namespace
  Fixed the last references to security.yml
  Updated the main security article
  Updated security/* articles to Symfony 4
  (WIP) Updated security/* articles to Symfony 4
  Fixing build problems
  tweaks thanks to @yceruto
  updating the rest of the Doctrine docs
  Update Custom UsernamePasswordToken Authenticator
  ...
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.

4 participants