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.

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

Add hiera defaults configuration options for all resources; rename $nginx_upstream_defaults to $nginx_upstreams_defaults #1080

Merged
merged 1 commit into from
Feb 9, 2019
Merged

Add hiera defaults configuration options for all resources; rename $nginx_upstream_defaults to $nginx_upstreams_defaults #1080

merged 1 commit into from
Feb 9, 2019

Conversation

mvisonneau
Copy link
Contributor

  • Added hiera defaults configuration options for all resources
  • Sorted variables
  • Casted them to an hash

@oranenj
Copy link
Contributor

oranenj commented Apr 22, 2017

Should there perhaps be new tests related to this change?

Either way, looks good to me.

@oranenj oranenj added the enhancement New feature or request label Apr 22, 2017
@wyardley
Copy link
Collaborator

I think this is partially covered by #1071 and similar PRs; aside from lack of tests, I think some of these resources may already have defaults that I think would get overwritten by these, e.g., https://github.com/voxpupuli/puppet-nginx/blob/master/manifests/resource/server.pp#L447-L453 (which is handled in #1071).

@wyardley wyardley added needs-feedback Further information is requested needs-tests labels Apr 22, 2017
@oranenj
Copy link
Contributor

oranenj commented Apr 30, 2017

I merged the other PR since it was smaller and had tests, so this will need a rebase for the remaining functionality.

@mvisonneau mvisonneau force-pushed the hiera_defaults branch 2 times, most recently from 3c7725d to 8e77df7 Compare May 30, 2017 10:54
@mvisonneau
Copy link
Contributor Author

👋 apologies for the late update. Just rebased and added some additional checks.

@wyardley
Copy link
Collaborator

wyardley commented Jun 1, 2017

@mvisonneau thanks for rebasing and adding tests. It seems as if some tests are failing in CI

@bastelfreak
Copy link
Member

Hi @mvisonneau, are you still interested in this PR? If so, please rebase against latest master. Afterwards we can check if travis is still failing.

@mvisonneau mvisonneau force-pushed the hiera_defaults branch 2 times, most recently from 3eb5171 to dc99ae4 Compare November 21, 2018 19:03
@bastelfreak bastelfreak changed the title Added hiera defaults configuration options for all resources Add hiera defaults configuration options for all resources Feb 9, 2019
@bastelfreak bastelfreak removed needs-feedback Further information is requested needs-rebase tests-fail labels Feb 9, 2019
@bastelfreak bastelfreak merged commit 07c9dd1 into voxpupuli:master Feb 9, 2019
alexjfisher
alexjfisher reviewed Feb 9, 2019
View reviewed changes
manifests/init.pp
create_resources( 'nginx::resource::map', $string_mappings, $string_mappings_defaults )
create_resources( 'nginx::resource::server', $nginx_servers, $nginx_servers_defaults )
create_resources( 'nginx::resource::streamhost', $nginx_streamhosts, $nginx_streamhosts_defaults )
create_resources( 'nginx::resource::upstream', $nginx_upstreams, $nginx_upstreams_defaults )
Copy link
Member

Choose a reason for hiding this comment

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

As per #1309, good luck creating nginx::resource::upstream resources with defaults of type Nginx::UpstreamMemberDefaults...

@bastelfreak bastelfreak changed the title Add hiera defaults configuration options for all resources Add hiera defaults configuration options for all resources; rename $nginx_upstream_defaults to $nginx_upstreams_defaults Feb 9, 2019
@bastelfreak bastelfreak removed the enhancement New feature or request label Feb 9, 2019
@alexjfisher alexjfisher mentioned this pull request Feb 9, 2019
Merged
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this pull request Oct 19, 2020
@bastelfreak
Merge pull request voxpupuli#1080 from mvisonneau/hiera_defaults
b4b8a95 
Add hiera defaults configuration options for all resources
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexjfisher alexjfisher alexjfisher left review comments

@bastelfreak bastelfreak bastelfreak left review comments

Assignees
No one assigned
Labels
backwards-incompatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mvisonneau @oranenj @wyardley @bastelfreak @alexjfisher