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

DOCS: Update nginx configuration for SilverStripe 4 #7812

Merged
merged 2 commits into from Feb 8, 2018

Conversation

oddnoc
Copy link
Contributor

@oddnoc oddnoc commented Jan 29, 2018

Port nginx configuration changes from 3.

@dhensby
Copy link
Contributor

dhensby commented Jan 30, 2018

We'll get these merged up so we don't need a PR for it

@dhensby dhensby closed this Jan 30, 2018
@oddnoc
Copy link
Contributor Author

oddnoc commented Jan 30, 2018

@dhensby There’s a crucial difference for SilverStripe 4: the try_files directive uses index.php rather than main.php.

@dhensby dhensby reopened this Jan 30, 2018
@dhensby
Copy link
Contributor

dhensby commented Jan 30, 2018

ok, @oddnoc have you also updated the nginx wiki: https://www.nginx.com/resources/wiki/start/topics/recipes/silverstripe/ ?

@dhensby
Copy link
Contributor

dhensby commented Jan 30, 2018

OK, I've cherry-picked the commit and put it on the 4.0 branch so we don't get all those unrelated commits in this PR.

Please can you recheck this as I had to resolve some conflicts and they looked quite major and I just blindly accepted your changes

@oddnoc oddnoc force-pushed the configure-nginx-ss4 branch 2 times, most recently from 0ecf1d4 to 6159475 Compare January 31, 2018 00:51
@oddnoc oddnoc changed the base branch from 4.0 to 4 January 31, 2018 00:55
@oddnoc
Copy link
Contributor Author

oddnoc commented Jan 31, 2018

@dhensby I think I see part of the problem. I meant to use 4, not 4.0, as the base, and now I've updated the base. But I don't see where you cherry-picked a commit into 4.0 (or 4).

Hadn't thought about the wiki, so thanks for the reminder. But I'll be happy to update the nginx wiki once the SilverStripe project has settled on the sample config.

@dhensby
Copy link
Contributor

dhensby commented Feb 5, 2018

@dhensby I think I see part of the problem. I meant to use 4, not 4.0, as the base, and now I've updated the base. But I don't see where you cherry-picked a commit into 4.0 (or 4).

Right, but what's wrong with putting it on 4.0, is the change not applicable to 4.0 and only to 4.x?

Was there something wrong with the changes I had made to the PR?

Hadn't thought about the wiki, so thanks for the reminder. But I'll be happy to update the nginx wiki once the SilverStripe project has settled on the sample config.

Ok

@oddnoc
Copy link
Contributor Author

oddnoc commented Feb 6, 2018

Would you rather have the PR against the 4.0 branch? Happy to accommodate. The new try_files would apply equally to any 4.x, including 4.0.

@robbieaverill
Copy link
Contributor

For reference, we often ask for docs changes to go into the 4 branch because that's the branch that we use for generating docs.silverstripe.org. If you merge into 4.0 it will still end up in the 4 branch eventually, it'll just take longer to get there

SS4 uses /index.php so the location block and documentation need to also
use that.
@dhensby dhensby changed the base branch from 4 to 4.0 February 8, 2018 10:40
@dhensby dhensby merged commit 09f88c7 into silverstripe:4.0 Feb 8, 2018
@oddnoc oddnoc deleted the configure-nginx-ss4 branch February 8, 2018 18:41
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.

None yet

3 participants