Skip to content
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

allow adding custom mime types while still using the module defaults #1268

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

bryangwilliam
Copy link
Contributor

Pull Request (PR) description

Allows users to add custom mime types to mime.types without requiring them to manage the full list. It will append any custom types to the default list rather than completely overriding it.

This Pull Request (PR) fixes the following issues

n/a

@othalla
Copy link

othalla commented Oct 20, 2018

This is a bit weird, why not explicitely specify all Mime types?
It is a bit weird to have additionals values while setting the $mime_types variable.
I prefer to have a second var like preserve_default_mime_types of something like that.

@bastelfreak
Copy link
Member

I'm not sure if the merge approach is a good idea, this might confuse users. I agree with @othalla that this should be another variable or it should be handled in a profile.

@bryangwilliam
Copy link
Contributor Author

bryangwilliam commented Oct 20, 2018

To clarify, are you saying that you would prefer passing in the customizations in the normal parameter and then have a second parameter that triggers a merge with the defaults if requested? I agree that seems like a better approach and have no problem switching, just want to make sure I understand before I make the changes.

I'd prefer to avoid using a profile because that means I either need to forsake the defaults entirely or I need to include nginx::params to access the defaults which feels a little weird (I am currently using the approach and my attempt to clean that up is what initiated the PR). I'm not completely against the approach and could be convinced to keep using it if we can't find a good solution, but it does feel a little hacky.

@othalla
Copy link

othalla commented Oct 20, 2018

Yep that's what i mean :)

@othalla
Copy link

othalla commented Oct 22, 2018

This is better :) should be ok

@bastelfreak bastelfreak added the enhancement New feature or request label Oct 24, 2018
@bastelfreak
Copy link
Member

Thanks!

@bastelfreak bastelfreak merged commit ae2af80 into voxpupuli:master Oct 24, 2018
@bryangwilliam bryangwilliam deleted the custom_mime_types branch December 22, 2018 20:26
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Sep 13, 2019
allow adding custom mime types while still using the module defaults
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this pull request Oct 19, 2020
allow adding custom mime types while still using the module defaults
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants