Closed potsky closed 7 months ago
Thanks for your pull request to Laravel!
Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.
If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!
If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.
I understand 👍 We will fork sail.
Hello @taylorotwell
we've forked for now, but I really think that the few lines in this PR are an advantage for the Laravel ecosystem. Sail is great for getting started on development quickly but it runs perfectly for several projects in large teams in the Docker style.
We should keep the Docker principle of independency of apps.
You had set APP_SERVICE
in the first commit to enable you to start several apps at the same time, but the job was only half done.
If you don't do the same thing for databases, for selenium... then the sail
command is half usable, it is not at the level of the Laravel ecosystem I think. You cannot run sail psql
, sail dusk
, sail redis
...
The 5 added lines simply take the container's name preferences, and if there are none, the default names are used.
It would really help to be able to specify the name of all the services :pray:
This PR let user choose the name of docker services instead of defaults (as already done for
APP_SERVICE
)We use several laravel apps communicating together via an API. Developers run these apps at the same time on their machines and they need to specify distinct names for databases, redis, ... in their env. eg:
app_laravel
cms_laravel
crm_laravel
app_redis
cms_redis
crm_redis
On each project, this PR let you set these variables in your
.env
file. eg: