8000 [FrameworkBundle] consolidate the mime types service definition by xabbuh · Pull Request #30001 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[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.

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

[FrameworkBundle] consolidate the mime types service definition #30001

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 •
edited
Loading

👎 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 commented Feb 5, 2019
View reviewed changes
@xabbuh xabbuh force-pushed the pr-29896-default-initialisation branch from 882f5b8 to c5ef9d9 Compare February 8, 2019 13:01
@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
@fabpot
minor #30001 [FrameworkBundle] consolidate the mime types service def…
b9b8f9d 
…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
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
5 participants
@xabbuh @nicolas-grekas @fabpot @chalasr @carsonbot
0