Skip to content

[FrameworkBundle] Allow to disable the PropertyAccessor and annotations #17706

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

[FrameworkBundle] Allow to disable the PropertyAccessor and annotations #17706

wants to merge 6 commits into from

Conversation

GuilhemN
Copy link
Contributor

@GuilhemN GuilhemN commented Feb 5, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? -
Fixed tickets #13703 (comment)
License MIT

Includes #17690

This PR allows to disable framework.property_access and framework.annotations and deprecate automatically enabling them.

@GuilhemN GuilhemN mentioned this pull request Feb 5, 2016
Closed
@GuilhemN
Copy link
Contributor Author

GuilhemN commented Feb 5, 2016

I'm waiting for feedbacks before fixing all deprecation notices.

xabbuh
xabbuh reviewed Feb 9, 2016
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
->always(function ($v) {
if (!isset($v['property_access'])) {
$v['property_access']['enabled'] = true;
@trigger_error('You must explicitly define wheter to enable "framework.property_access" or not. It will be disabled by default in 4.0', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

not sure if that's a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed #13703, it is confusing to have some components enabled by default and some others disabled by default.
BTW this deprecation can be easily fixed in the symfony standard edition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I commented it for now as there are a lot of changes to make to fix all the deprecations in the core then ...

@fabpot
Copy link
Member

fabpot commented Mar 2, 2016

This one should be rebased once #17690 is merged.

@GuilhemN GuilhemN force-pushed the CONFIG branch 2 times, most recently from e9a2a36 to cbf2211 Compare March 2, 2016 13:39
@GuilhemN
Copy link
Contributor Author

GuilhemN commented Mar 2, 2016

@fabpot I rebased this PR and commented the deprecations notices for now as it make a lot of deprecations to fix...
I'm wondering if this PR should deprecate enabling property_access and annotations by default or if we should do that later.

@GuilhemN
Copy link
Contributor Author

I removed the deprecations to have something easier to review as we can still add them later.

@GuilhemN
Copy link
Contributor Author

I'm closing this for now as the conflicts are too complex to fix.

@GuilhemN GuilhemN closed this Jun 23, 2016
@GuilhemN GuilhemN deleted the CONFIG branch June 23, 2016 16:39
@GuilhemN GuilhemN mentioned this pull request Jun 23, 2016
Merged
fabpot added a commit that referenced this pull request Jun 24, 2016
@fabpot
minor #19155 [FrameworkBundle] Remove redundant code (Ener-Getick)
358a4cc 
This PR was merged into the 3.2-dev branch.

Discussion
----------

[FrameworkBundle] Remove redundant code

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

Was part of #17706.
Change the remaining ``isset($config['foo'])`` to ``$this->isConfigEnabled($config['foo'])`` to allow to use parameters to enable a feature.

Commits
-------

a21af88 [FrameworkBundle] Remove redundant code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@GuilhemN @fabpot @xabbuh @javiereguiluz @carsonbot