Skip to content

Allow configuring class names through methods instead of class parameters in Doctrine extensions #33319

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

alcaeus
Copy link
Contributor

@alcaeus alcaeus commented Aug 24, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT
Doc PR symfony/symfony-docs#...

While removing class parameters for DoctrineBundle 2.0 (see doctrine/DoctrineBundle#630), I noticed that the DoctrineExtension still requires them. This PR adds a new method that keeps legacy behaviour, but will dropped in Symfony 5. Extending classes (mainly DoctrineBundle and DoctrineMongoDBBundle) must implement this method themselves to return the appropriate class names instead of declaring them as class parameters in their service configuration. I'll create a separate for the master branch to make this method abstract in 5.0.

The cache driver class names are not being replaced in this PR, as we're dropping support for doctrine/cache in DoctrineBundle 2.0. A separate PR will be created to handle those deprecations and to clean up the code.

@nicolas-grekas nicolas-grekas added this to the next milestone Aug 26, 2019
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Can you add the type hints and removed the phpdocs? Thank you.

@alcaeus alcaeus force-pushed the doctrine-bridge-make-class-params-obsolete branch from d481173 to b53d8cc Compare August 27, 2019 07:49
@alcaeus
Copy link
Contributor Author

alcaeus commented Aug 27, 2019

@fabpot @OskarStark done. Sorry for the delay.

@OskarStark
Copy link
Contributor

@fabpot @OskarStark done. Sorry for the delay.

No problem 😃

@fabpot
Copy link
Member

fabpot commented Aug 27, 2019

Thank you @alcaeus.

fabpot added a commit that referenced this pull request Aug 27, 2019
…of class parameters in Doctrine extensions (alcaeus)

This PR was merged into the 4.4 branch.

Discussion
----------

Allow configuring class names through methods instead of class parameters in Doctrine extensions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

While removing class parameters for DoctrineBundle 2.0 (see doctrine/DoctrineBundle#630), I noticed that the DoctrineExtension still requires them. This PR adds a new method that keeps legacy behaviour, but will dropped in Symfony 5. Extending classes (mainly DoctrineBundle and DoctrineMongoDBBundle) must implement this method themselves to return the appropriate class names instead of declaring them as class parameters in their service configuration. I'll create a separate for the master branch to make this method abstract in 5.0.

The cache driver class names are not being replaced in this PR, as we're dropping support for `doctrine/cache` in DoctrineBundle 2.0. A separate PR will be created to handle those deprecations and to clean up the code.

Commits
-------

b53d8cc [DoctrineBridge] Allow configuring class names through methods instead of class parameters
@fabpot fabpot merged commit b53d8cc into symfony:4.4 Aug 27, 2019
@alcaeus alcaeus deleted the doctrine-bridge-make-class-params-obsolete branch August 27, 2019 08:01
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
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.

5 participants