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

Add mime.types file template and default values for it #1243

Merged
merged 2 commits into from
Sep 7, 2018
Merged

Add mime.types file template and default values for it #1243

merged 2 commits into from
Sep 7, 2018

Conversation

martialblog
Copy link
Contributor

  • The mine.types file can now be configured using a simple hash

Pull Request (PR) description

When using this module to only configure nginx and then run in a Container, we ran into an issue when we tried to store the config in a different directory. The config directory can be adjusted using the conf_dir parameter, however the module does not provide a mime.types file.

This PR fixes this issue by adding a simple mime.types.erb that can be filled with a hash, and some default values for the file.

This Pull Request (PR) fixes the following issues

Fixes #1240

@@ -147,6 +147,7 @@
$package_source = 'nginx',
$package_flavor = undef,
$manage_repo = $nginx::params::manage_repo,
$mime_types = $nginx::params::mime_types,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a datatype for the new parameter. Probably Hash[String[1], String[1]]

<% @mime_types.each do |key, value| -%>
<%= key %> <%= value %>;
<% end -%>
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a trailing newline

@@ -0,0 +1,6 @@
# MANAGED BY PUPPET
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please convert this to an epp template.

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels Sep 3, 2018
@martialblog
Copy link
Contributor Author

@bastelfreak Hi, I'll take care of it

 - The mine.types file can now be configured using a simple hash
@martialblog
Copy link
Contributor Author

I addressed the issues, but the Beaker tests blew up O_o

types {
<% $nginx::mime_types.each |$key, $value| { -%>
<%= $key %> <%= $value %>;
<% } -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the - I guess.

<% $nginx::mime_types.each |$key, $value| { -%>
<%= $key %> <%= $value %>;
<% } -%>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not an empty newline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To solve a bit of the confusion I might have created. A quote from our review docs:

  • Files should always terminate with a newline if possible, with an exception being file or template fragments like those used with concat. This is the POSIX
    standard
    , and some tools don't handle the lack of a terminating newline properly

@martialblog
Copy link
Contributor Author

@bastelfreak Ok, thanks. I'm not too familiar with EPP Templates yet.

@martialblog
Copy link
Contributor Author

@bastelfreak Hi, should be fine now.

cat -A templates/conf.d/mime.types.epp                                                                              [11:14:57]
# MANAGED BY PUPPET$
types {$
<% $nginx::mime_types.each |$key, $value| { %>$
    <%= $key %> <%= $value %>;$
<% } %>$
}$

@@ -147,6 +147,7 @@
$package_source = 'nginx',
$package_flavor = undef,
$manage_repo = $nginx::params::manage_repo,
Hash[String[1], String[1]] $mime_types = $nginx::params::mime_types,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datatypes \o/

@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Sep 7, 2018
@bastelfreak
Copy link
Member

Thanks!

@bastelfreak bastelfreak merged commit 78bd190 into voxpupuli:master Sep 7, 2018
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Sep 13, 2019
Add mime.types file template and default values for it
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this pull request Oct 19, 2020
Add mime.types file template and default values for it
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.

Reliance on mime.types
2 participants