symfony/symfony-docsPublic

NotificationsYou must be signed in to change notification settings
Fork5.3k
Star2.3k

Update heroku.rst#6750

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

sherif807 wants to merge1 commit intosymfony:3.1fromsherif807:patch-1

Closed

Update heroku.rst#6750

sherif807 wants to merge1 commit intosymfony:3.1fromsherif807:patch-1

Conversation

Copy link

sherif807 commentedJul 13, 2016

changed web: bin/heroku-php-apache2 web/ to web: vendor/bin/heroku-php-apache2 web/

Update heroku.rst

ad8e6e2

changed web: bin/heroku-php-apache2 web/    to    web: vendor/bin/heroku-php-apache2 web/

carsonbot added the Status: Needs Review label

Jul 13, 2016

Copy link

Member

xabbuh commentedJul 23, 2016

👍

Status: Reviewed

Note to merger: This should be merged into the3.0 branch.

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 23, 2016

Copy link

Member

weaverryan commentedAug 2, 2016

Hi@sherif807!

I was about to merge this (because the fix is correct), but I noticed that a little bit more needs to be fixed. Earlier in this section (https://github.com/sherif807/symfony-docs/blob/ad8e6e29e2fd66e94e942ac3b555151d79f9d84e/cookbook/deployment/heroku.rst#1-create-a-procfile) - it says this:

The Composer bin-dir, where vendor binaries (and thus Heroku's own boot scripts) are placed, is bin/ , and not the default vendor/bin.

Obviously, in Symfony 3.0, this is not true! So, can we remove this note? Also, I believe the 2 code-blocks above this change also need to be changed to point tovendor/bin/.. instead of justbin/....

If you agree, could you make those changes?

Status: Needs Work

carsonbot added Status: Needs Work and removed Status: Reviewed labels

Aug 2, 2016

Copy link

Member

javiereguiluz commentedNov 22, 2016

Closing it in favor of#7155.@sherif807 thanks for proposing this fix in the first place!

javiereguiluz closed this

Nov 22, 2016

javiereguiluz mentioned this pull request

Nov 22, 2016

Updated the Heroku deployment article#7155

Merged

xabbuh added a commit that referenced this pull request

Nov 22, 2016

minor#7155Updated the Heroku deployment article (javiereguiluz)

63696f9

This PR was merged into the 3.1 branch.Discussion----------Updated the Heroku deployment articleThis finishes#6750 with the changes proposed by@weaverryan. I'm pinging@dzuelke to kindly ask him to review of these changes before merging them. Thanks!Commits-------815bb81 Updated the Heroku deployment article