Skip to content

[DependencyInjection] Deprecated passing Parameter instances as class name to Definition #32390

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

derrabus
Copy link
Member

@derrabus derrabus commented Jul 5, 2019

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

This PR deprecates the undocumented possibility to use a Parameter instance instead of a string as class name for a Definition. This was discovered while working on #32266.

@derrabus derrabus force-pushed the improvement/class-parameter-deprecation branch from a800e12 to 853efd1 Compare July 5, 2019 10:07
@derrabus derrabus changed the title Deprecated passing Parameter instances as class name to Definition. Deprecated passing Parameter instances as class name to Definition Jul 5, 2019
@derrabus derrabus changed the title Deprecated passing Parameter instances as class name to Definition [DependencyInjection] Deprecated passing Parameter instances as class name to Definition Jul 5, 2019
@derrabus
Copy link
Member Author

derrabus commented Jul 5, 2019

Failure on AppVeyor is unrelated.

@derrabus derrabus force-pushed the improvement/class-parameter-deprecation branch from 853efd1 to 510fa1f Compare July 5, 2019 14:27
@derrabus
Copy link
Member Author

derrabus commented Jul 5, 2019

Status: Needs Review

@derrabus derrabus force-pushed the improvement/class-parameter-deprecation branch from 510fa1f to 11a27fd Compare July 5, 2019 18:40
@derrabus
Copy link
Member Author

derrabus commented Jul 5, 2019

Staus: Needs Review

@Tobion
Copy link
Contributor

Tobion commented Jul 5, 2019

Deprecation warning, see https://travis-ci.org/symfony/symfony/jobs/554806241
The test is also failing in #32266 and needs to be fixed in a lower branch.

@derrabus derrabus force-pushed the improvement/class-parameter-deprecation branch from 11a27fd to f4af95f Compare July 5, 2019 19:05
@derrabus derrabus force-pushed the improvement/class-parameter-deprecation branch from f4af95f to edfc9d6 Compare July 5, 2019 19:06
@derrabus
Copy link
Member Author

derrabus commented Jul 5, 2019

Deprecation warning

Fixed. I'm going to apply the fix on 4.2 in another PR.

@Tobion
Copy link
Contributor

Tobion commented Jul 5, 2019

Thank you @derrabus.

@Tobion Tobion merged commit edfc9d6 into symfony:4.4 Jul 5, 2019
Tobion added a commit that referenced this pull request Jul 5, 2019
…tances as class name to Definition (derrabus)

This PR was merged into the 4.4 branch.

Discussion
----------

[DependencyInjection] Deprecated passing Parameter instances as class name to Definition

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

This PR deprecates the undocumented possibility to use a `Parameter` instance instead of a string as class name for a `Definition`. This was discovered while working on #32266.

Commits
-------

edfc9d6 Deprecated passing Parameter instances as class name to Definition.
@derrabus
Copy link
Member Author

derrabus commented Jul 5, 2019

Thanks for the review, @Tobion. Here's the PR for the test: #32399

@derrabus derrabus deleted the improvement/class-parameter-deprecation branch July 5, 2019 19:24
Tobion added a commit that referenced this pull request Jul 5, 2019
…rBuilder::register (derrabus)

This PR was merged into the 4.2 branch.

Discussion
----------

[Messenger] Don't pass objects as class name to ContainerBuilder::register

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

Fixed a broken test that was discovered while working on #32390.

Commits
-------

e110603 Don't pass objects as class name to ContainerBuilder::register.
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.

4 participants