Skip to content

[FrameworkBundle] Remove redundant code #19155

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
Merged
fabpot merged 1 commit into from
Jun 24, 2016
Merged

[FrameworkBundle] Remove redundant code #19155

fabpot merged 1 commit into from
Jun 24, 2016

Conversation

@GuilhemN
Copy link
Contributor

@GuilhemN GuilhemN commented Jun 23, 2016

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.

@stof
Copy link
Member

stof commented Jun 23, 2016

The fact that it does not change the Configuration class looks weird to me

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Jun 23, 2016

@stof canBeEnabled is used in the Configuration so it's ok. In fact it's possible that this was broken, i'll run some checks.

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Jun 23, 2016

@stof ok in fact the $this->isConfigEnabled() is inside the register* functions.
Anyway this still looks inconsistent to me, imo we should move this checks to the main function, wdyt?

@xabbuh
Copy link
Member

xabbuh commented Jun 23, 2016

At least, I would now remove the second call to the isConfigEnabled() method inside registerSerializerConfiguration() and registerPropertyInfoConfiguration().

@GuilhemN GuilhemN force-pushed the FRAMEWORKEXTENSION branch from a02ee60 to c30f0b0 Compare June 23, 2016 20:21
@GuilhemN GuilhemN changed the title [FrameworkBundle] Fix small inconsistensies [FrameworkBundle] Fix redundant code Jun 23, 2016
@GuilhemN GuilhemN changed the title [FrameworkBundle] Fix redundant code [FrameworkBundle] Remove redundant code Jun 23, 2016
@GuilhemN GuilhemN force-pushed the FRAMEWORKEXTENSION branch from c30f0b0 to a21af88 Compare June 23, 2016 20:21
@GuilhemN
Copy link
Contributor Author

GuilhemN commented Jun 23, 2016

done @xabbuh

@xabbuh
Copy link
Member

xabbuh commented Jun 23, 2016

👍 LGTM

@fabpot
Copy link
Member

fabpot commented Jun 24, 2016

Thank you @Ener-Getick.

@fabpot fabpot merged commit a21af88 into symfony:master Jun 24, 2016
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
@GuilhemN GuilhemN deleted the FRAMEWORKEXTENSION branch June 24, 2016 06:23
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

FrameworkBundle Status: Needs Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@GuilhemN @stof @xabbuh @fabpot @carsonbot