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

moves rewrite_rules to location_header #526

Merged
merged 1 commit into from
Dec 11, 2014
Merged

moves rewrite_rules to location_header #526

merged 1 commit into from
Dec 11, 2014

Conversation

paschdan
Copy link

in some vhosts i need to have rewrites inside fastcgi locations.

this PR adds rewrite rules to the fastcgi.erb file.

for the specs i had to use regex to get the tests going, i am not sure how to write the test in better format.

@jfryman
Copy link
Contributor

jfryman commented Dec 11, 2014

Awesome. I think this should be ok.

@3flex whaddya think?

@3flex
Copy link
Contributor

3flex commented Dec 11, 2014

I think it's a good addition, but it should go in templates/vhost/location_header.erb instead of just the FastCGI template so it can be used for all location types. Though now I look I see the same code in templates/vhost/locations/directory.erb and templates/vhost/locations/proxy.erb...

@paschdan would you be comfortable removing it from directory.erb and proxy.erb and moving it to location_header.erb? If not, then @jfryman I say merge as is and we can look at tidying that up later.

@paschdan
Copy link
Author

@3flex i could do that, yes

@3flex
Copy link
Contributor

3flex commented Dec 11, 2014

Sweet, the templates look good. One last thing - can you please move the test up to the section starting around line 35? That way it tests the header template, not just the fastcgi template output. Then squash commits to one if you could.

Thanks for the contribution!

so it can be used by all location-types
@paschdan paschdan changed the title adds rewrites to fastcgi locations moves rewrite_rules to location_header Dec 11, 2014
@paschdan
Copy link
Author

@3flex here you go

jfryman added a commit that referenced this pull request Dec 11, 2014
moves rewrite_rules to location_header
@jfryman jfryman merged commit 10d5f19 into voxpupuli:master Dec 11, 2014
@jfryman
Copy link
Contributor

jfryman commented Dec 11, 2014

Excellent stuff. Thanks again!

@paschdan paschdan deleted the rewrites_in_fastcgi branch January 19, 2015 16:24
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
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

Successfully merging this pull request may close these issues.

3 participants