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

Use attribute for listen port in nginx default server template #219

Merged
merged 2 commits into from Mar 14, 2015

Conversation

@jeffbyrnes
Copy link

commented May 15, 2014

Deals with the same issue that #201 offers to provide, but following Chef convention more closely (as per @eherot’s comment on the abovementioned PR).

@JamesBelchamber

This comment has been minimized.

Copy link

commented Jun 12, 2014

Tested, working - can we get this merged please? :D

@JamesBelchamber

This comment has been minimized.

Copy link

commented Jun 12, 2014

@@ -25,6 +25,7 @@
# attributes by modifying a role, or the node itself.
default['nginx']['version'] = '1.4.4'
default['nginx']['package_name'] = 'nginx'
default['nginx']['port'] = '8080'

This comment has been minimized.

Copy link
@pangratz

pangratz Jun 17, 2014

Wouldn't this be a change, which breaks backwards compatibility? Currently the used port for the default page is 80, which would be changed to 8080 by this ...

Apart from that, I am 👍 on making the default port be read from an attribute.

This comment has been minimized.

Copy link
@jeffbyrnes

jeffbyrnes Jun 17, 2014

Author

@pangratz good catch, I'll fix that up.

@jeffbyrnes

This comment has been minimized.

Copy link
Author

commented Aug 11, 2014

I'm sure @JamesBelchamber & many others would appreciate this merging in.

@miketheman miketheman added this to the Triage milestone Aug 23, 2014
@miketheman miketheman self-assigned this Aug 23, 2014
@jeffbyrnes

This comment has been minimized.

Copy link
Author

commented Oct 1, 2014

Bump.

@miketheman

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2014

@jeffbyrnes @JamesBelchamber Thanks for the contribution! We're looking at getting a lot of bugs fixed before proceeding to enhancements, and they need more testing to ensure we deliver a solid release.
Tried to make that clear in the roadmap - does that make sense?

@jeffbyrnes

This comment has been minimized.

Copy link
Author

commented Oct 2, 2014

@miketheman makes total sense. Apologies for forgetting about the roadmap; I did see that you'd established that since this PR was put in place.

I'll see if I can help out with bug squashing.

@miketheman miketheman modified the milestones: 2.7.5, Triage Round 1 Mar 14, 2015
miketheman added a commit that referenced this pull request Mar 14, 2015
Use attribute for listen port in nginx default server template
@miketheman miketheman merged commit 0b77287 into sous-chefs:master Mar 14, 2015
@jeffbyrnes jeffbyrnes deleted the evertrue:add_port_variable_to_template branch Mar 16, 2015
@tas50 tas50 removed the enhancement label Jan 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.