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

[8.x] Allow symlink recreation in storage:link command #37501

Merged
merged 3 commits into from
May 27, 2021

Conversation

netpok
Copy link
Contributor

@netpok netpok commented May 27, 2021

This PR allows symlinks to be overridden when --force option is specified to the storage:link command. Existing behavior was not changed and regular files won't be replaced with symlinks.

This could come helpful:

  • When project location is changed but the original location is still valid
  • Symlink target location changed but the original location is still valid

Currently the only solution to these cases is to manually remove the existing symlinks

Co-authored-by: Dries Vints <dries@vints.io>

foreach ($this->links() as $link => $target) {
if (file_exists($link)) {
if (file_exists($link) && ! (is_link($link) && $force)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you rewrite this conditional? It is hard to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this better?

* @param bool $force
* @return bool
*/
protected function removableSymlink(string $link, bool $force): bool
Copy link
Member

Choose a reason for hiding this comment

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

Boolean methods often are prefixed with is or has:

Suggested change
protected function removableSymlink(string $link, bool $force): bool
protected function isRemovableSymlink(string $link, bool $force): bool

@taylorotwell taylorotwell merged commit e4b509e into laravel:8.x May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants