sort $vhost_cfg_append hash in vhost_footer.erb template #115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you please rebase and re-submit? Thanks! |
There were some bugs in the existing $::operatingsystem based approach. * amazon was it's own package set when it's properly part of $::osfamily == 'redhat' as of facter >= 1.7.2 * gentoo was improperly part of the amazon package set; this patch removes support for gentoo but it was broken anyways modifications to nginx::package::redhat were made as well * it no longer tries to setup the nginx.org yumrepo for fedora as no packages for fedora are currently provided * amazon release numbers are inconsistent with EL. Unknown $::lsbmajdistrelease values are now mapped to 6 so it's no longer nessicary to test for $::lsbmajdistrelease being undefined. This logic will need to be reworked after RHEL7.x is released. * the url to the nginx repo was including $::operatingsystem in it but nginx.org only has package dirs for 'rhel' & 'centos' which are presently identical; the usage of the 'rhel' dir has been hardcoded. This fixes broken yum repo setup for all $::osfamily == 'redhat' platforms other than redhat and centos.
|
Done. |
|
@jfryman Any thoughts on my question if that other templates should be modified in a similar way or if the API should be changed so directives can be ordered? I'm at puppetconf if you want to have a 30s face to face on this. :) |
|
This PR was apparently a train wreck. Somehow the commit from this PR #99 ended up on this topic branch and the intended commit never got pushed to github. |
|
I've opened a new PR #123 for the changeset that was actually intended. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is so the paremter ordering in the configuration is stable. It also
force the :allow key (if it exists) to be first in the sort ordering.
It would probably be better to change
$vhost_cfg_appendinto an arrayof one key hashes but would require a public API change.
I'm not intending this PR to actually get merged as several other defines have similar problems. I'd like to know if they should all be fixed in a similar, if a bit ugly, manner or if a PR to change the API would be accepted.