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

[FrameworkBundle] consolidate the mime types service definition #30001

Merged
merged 1 commit into from Feb 12, 2019

Conversation

Projects
None yet
5 participants
@xabbuh
Copy link
Member

xabbuh commented Jan 27, 2019

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

I wonder if we couldn't simplify things by just initializing the default MimeTypes instance once the first one is going to be created.

@xabbuh xabbuh added this to the next milestone Jan 27, 2019

@xabbuh xabbuh added the Mime label Jan 27, 2019

@xabbuh xabbuh force-pushed the xabbuh:pr-29896-default-initialisation branch from a6a5fab to 8abf6ce Jan 27, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 27, 2019

👎 bad idea, this shouldn't be that contextual IMHO.

@xabbuh xabbuh force-pushed the xabbuh:pr-29896-default-initialisation branch from 8abf6ce to 882f5b8 Feb 5, 2019

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Feb 5, 2019

Okay, makes sense somehow. But I think we should then move the setDefault() call to the service definition itself to make it more discoverable how it is actually wired at runtime.

@xabbuh xabbuh changed the title [Mime] initialize the default MimeTypes instance in its constructor [FrameworkBundle] initialize the default MimeTypes instance in its constructor Feb 5, 2019

@xabbuh xabbuh changed the title [FrameworkBundle] initialize the default MimeTypes instance in its constructor [FrameworkBundle] consolidate the mime types service definition Feb 5, 2019

@xabbuh xabbuh added FrameworkBundle and removed Mime labels Feb 5, 2019

@xabbuh xabbuh force-pushed the xabbuh:pr-29896-default-initialisation branch from 882f5b8 to c5ef9d9 Feb 8, 2019

@xabbuh xabbuh force-pushed the xabbuh:pr-29896-default-initialisation branch from c5ef9d9 to ce546f5 Feb 8, 2019

@fabpot

fabpot approved these changes Feb 12, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Feb 12, 2019

Thank you @xabbuh.

@fabpot fabpot merged commit ce546f5 into symfony:master Feb 12, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Feb 12, 2019

minor #30001 [FrameworkBundle] consolidate the mime types service def…
…inition (xabbuh)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[FrameworkBundle] consolidate the mime types service definition

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

I wonder if we couldn't simplify things by just initializing the default `MimeTypes` instance once the first one is going to be created.

Commits
-------

ce546f5 consolidate the mime types service definition

@xabbuh xabbuh deleted the xabbuh:pr-29896-default-initialisation branch Feb 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment