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.

Sign up for GitHub

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

Jump to bottom

Fix usage of none for services list #495

Merged
merged 3 commits into from
Nov 21, 2022
Merged

Fix usage of none for services list #495

merged 3 commits into from
Nov 21, 2022

Conversation

jf-prevost
Copy link
Contributor

php artisan sail:install --with=none not handled properly

the none option leaves an empty $services array.
However this array is not checked before trying to pull related docker images before the build.
The fix checks whether $services has some entries, if not skips image pulling process and handles build of the container.

jf-prevost and others added 2 commits November 19, 2022 17:22
@jf-prevost
fix issue #492
3ba62ee 
php artisan sail:install --with=none not handled properly
@driesvints
Update InstallCommand.php
c4b13ca
driesvints
driesvints approved these changes Nov 21, 2022
View reviewed changes
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

I feel like we should accept this as right now the none option doesn't work properly.

@driesvints driesvints changed the title fix issue #492 (php artisan sail:install --with=none Fix usage of none for services list Nov 21, 2022
@driesvints driesvints linked an issue Nov 21, 2022 that may be closed by this pull request
Laravel Sail installation in the project fails when using --with=none #492
Closed
@taylorotwell taylorotwell merged commit 0dbee88 into laravel:1.x Nov 21, 2022
@jf-prevost jf-prevost deleted the fix-issue-492 branch November 21, 2022 17:27
@askmrsinh askmrsinh mentioned this pull request Jun 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driesvints driesvints driesvints approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Laravel Sail installation in the project fails when using --with=none
3 participants
@jf-prevost @driesvints @taylorotwell