Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[FrameworkBundle] consolidate the mime types service definition #30001

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
merged 1 commit into from
Feb 12, 2019

Conversation

xabbuh
Copy link
Member

@xabbuh 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 pr-29896-default-initialisation branch from a6a5fab to 8abf6ce Compare January 27, 2019 11:41
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 27, 2019

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

@xabbuh xabbuh force-pushed the pr-29896-default-initialisation branch from 8abf6ce to 882f5b8 Compare February 5, 2019 19:20
@xabbuh
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 force-pushed the pr-29896-default-initialisation branch from c5ef9d9 to ce546f5 Compare February 8, 2019 13:33
@fabpot
Copy link
Member

fabpot commented Feb 12, 2019

Thank you @xabbuh.

@fabpot fabpot merged commit ce546f5 into symfony:master Feb 12, 2019
fabpot added a commit that referenced this pull request Feb 12, 2019
…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 pr-29896-default-initialisation branch February 12, 2019 07:51
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.