-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Custom bin #608
Custom bin #608
Conversation
All in all it looks good to me but would you really download composer if it's missing? I think that should only rise an exception and stop deployer. And maybe deployer should test for a few common locations of composer before doing that. I really don't like the idea, that a application loads extra software without warning or at least an info. |
For now composer automatically downloaded too. If we remove this, may broke BC. |
Also it's downloaded via |
Yep, unfortunately, but maybe we can keep this topic on the agenda for a next major release. |
👍
|
|
||
if(empty($composer)) { | ||
run("cd {{release_path}} && curl -sS https://getcomposer.org/installer | php"); | ||
$composer = 'php composer.phar'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace with {{bin/php}} composer.phar
$composer = env()->parse('{{bin/php}} composer.phar');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Users can void a download
😄 |
Conflicts: recipe/common.php
@deployphp/maintainer merge? :) |
I think we will merge to both 3.1.x and 4.x |
@Elfet I merged branch |
I think merge master to 4.x too :) |
Yeah, I merged 😄 |
@antonmedv would it be easy to change the php bin per host? Our use-case: we would like to start experimenting with PHP7.4 without affecting the other applications on that host. |
composer_command
This is my version of custom bins. Here an example:
What do you think, guys?