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

nginx::resource::location include parameter is undocumented, and the generated include directive is formatted strangely #976

Open
srinchiera opened this issue Nov 17, 2016 · 2 comments

Comments

@srinchiera
Copy link

srinchiera commented Nov 17, 2016

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: all
  • Ruby: all
  • Distribution: all
  • Module version: latest (commit 912a2b9305d15793235560dfbfbf0b93e1a0077f)

How to reproduce (e.g Puppet code you use)

To reproduce weird formatting of the include directive...

     nginx::resource::location{ '/my_endpoint':
        location_custom_cfg => { "default_type" => "text/plain"},
        include => ["my-files/*"]
    }

What are you seeing

  1. The include parameter does not appear in the doctring for nginx::resource::location https://github.com/voxpupuli/puppet-nginx/blob/master/manifests/resource/location.pp

  2. The include directive generated by nginx::resource::location contains unnecessary leading spaces.

What behaviour did you expect instead

  1. I expect the include parameter to be documented like the other parameters.
  2. I expect there to be no unnecessary leading spaces in the include directive

Additional information

Perhaps this variable should be named include_files like it is in nginx::resource::vhost?

srinchiera pushed a commit to srinchiera/puppet-nginx that referenced this issue Nov 17, 2016
@wyardley
Copy link
Collaborator

@srinchiera: thanks, will review again, but I think this looks good!

If you want to rename it include_files (or, actually, since the directive is include, maybe we should rename it to just include in vhost?), I think that's probably also a good idea; we have some latitude for now to make breaking changes in service of consistency... if you put in another PR for that, I'll be happy to merge it (this module does have a confusing pattern where the plurals are confusing, e.g., fastcgi_param / fastcgi_params

wyardley added a commit that referenced this issue Nov 17, 2016
@TheMeier
Copy link
Contributor

This issue should be resolved.

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this issue Sep 13, 2019
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this issue Oct 19, 2020
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this issue Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants