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

[Cookbook] [Deployment] Added note about Nginx #5216

Merged
merged 1 commit into from
May 23, 2015

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Apr 28, 2015

Q A
Doc fix? yes
New docs? no
Applies to 2.3+
Fixed tickets

@@ -106,6 +106,15 @@ directory of the application and add just the following content:

web: bin/heroku-php-apache2 web/

.. note::

Nginx is also available in Heroku. If you prefer to use it, you can create a
Copy link
Member

Choose a reason for hiding this comment

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

on Heroku

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @xabbuh, fixed.

.. note::

Nginx is also available on Heroku. If you prefer to use it, you can create a
Nginx config file (``nginx_app.conf`` for instance) to point from Procfile as described in `Heroku docs`_:
Copy link
Member

Choose a reason for hiding this comment

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

I think we should reword the paragraph a bit like this:

If you prefer to use Nginx, which is also available on Heroku, you can create a configuration file for it and point to it from your Procfile as described in the Heroku documentation.

What do you think about this @phansys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sounds better. Updated.

@xabbuh
Copy link
Member

xabbuh commented Apr 29, 2015

👍 nice work @phansys

@phansys
Copy link
Contributor Author

phansys commented May 5, 2015

Should I rebase the PR and resolve the conflicts?

@phansys phansys force-pushed the deploy_heroku branch 3 times, most recently from 8ebb689 to c39f20e Compare May 5, 2015 18:00
@xabbuh
Copy link
Member

xabbuh commented May 6, 2015

@phansys That would be nice.

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets |
@phansys
Copy link
Contributor Author

phansys commented May 6, 2015

@xabbuh, done. Thanks.

@weaverryan
Copy link
Member

I love it! Thanks Javier!

@weaverryan weaverryan merged commit 751d0df into symfony:2.3 May 23, 2015
weaverryan added a commit that referenced this pull request May 23, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Cookbook] [Deployment] Added note about Nginx

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets |

Commits
-------

751d0df [Cookbook] [Deployment] Added note about Nginx setup
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.

4 participants