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

Updated Heroku instructions #6394

Merged
merged 3 commits into from
Apr 12, 2016
Merged

Conversation

magnusnordlander
Copy link
Contributor

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

With newer versions of the Heroku toolbelt, multiple buildpacks is supported natively.

With the newer versions of the Heroku toolbelt, multiple buildpacks is supported by default.
$ heroku buildpacks:set heroku/nodejs
Buildpack set. Next release on your-application will use heroku/nodejs.
Run git push heroku master to create a new release using this buildpack.
$ heroku buildpacks:set heroku/php --index 2
Copy link
Member

Choose a reason for hiding this comment

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

you could just use heroku buildpacks:add

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered that, but in the end I thought this was better because this way is idempotent.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't buildpacks:add added for exactly this usage (please note that I don't know anything about heroku)? If it is, I think we should use it instead of the current situation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sort of is, but the problem is that build pack order matters. In order to be able to use a composer post-install hook for e.g. Gulp, the node build pack needs to come before the PHP build pack. Just using buildpacks:add makes the process depend on the state of the application. The process I describe is idempotent.

use a special `multiple buildpack`_. To override buildpack auto-detection, you
need to explicitly set the buildpack URL:
now ignored. To compile your app with both buildpacks, Node.js *and* PHP, you need
use both buildpacks. To override buildpack auto-detection, you
Copy link
Member

Choose a reason for hiding this comment

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

[...] you need to use [...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@xabbuh
Copy link
Member

xabbuh commented Mar 31, 2016

👍 LGTM

use a special `multiple buildpack`_. To override buildpack auto-detection, you
need to explicitly set the buildpack URL:
now ignored. To compile your app with both buildpacks, Node.js *and* PHP, you need
to use both buildpacks. To override buildpack auto-detection, you
Copy link
Member

Choose a reason for hiding this comment

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

Effectively, you're saying "To use both builds packs, you need to use both buildpacks." That's kind of strange. What about changing this to "To compile your app with both buildpacks, override the buildpack auto-detection with the buildpacks:set command:"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rather use something like "To compile your app with both buildpacks, manually select them with the buildpacks:set command" (because the key concept here isn't that you're overriding the auto-detection, but rather that you're using multiple packs).

@weaverryan
Copy link
Member

Thanks Magnus!

@weaverryan weaverryan merged commit 204425c into symfony:2.3 Apr 12, 2016
weaverryan added a commit that referenced this pull request Apr 12, 2016
This PR was merged into the 2.3 branch.

Discussion
----------

Updated Heroku instructions

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

With newer versions of the Heroku toolbelt, multiple buildpacks is supported natively.

Commits
-------

204425c Fixed grammatical error
44e44f9 Comment updates
05a0afd Updated Heroku instructions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants