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

BUG Update installer to create the assets folder if its missing #8890

Conversation

maxime-rainville
Copy link
Contributor

If you start your project with composer create-project silverstripe/recipe-cms, the assets folder doesn't get created automatically. The installer falls on its face if the assets folder doesn't exists.

This PR simply makes that directory if it's missing and writeable.

Parent issue

@kinglozzer
Copy link
Member

Should this be wrapped in an if ($this->checkModuleExists('assets')) {? I realise there’s existing assets-related code in the installer that isn’t, just thought I’d raise it as a suggestion 😅

@ScopeyNZ
Copy link
Contributor

Is ASSETS_PATH even defined in framework? I would've thought it would be part of silverstripe-assets.

@dhensby
Copy link
Contributor

dhensby commented Mar 30, 2019

@maxime-rainville
Copy link
Contributor Author

maxime-rainville commented Mar 31, 2019

assets is defined as a dependency of framework.

"silverstripe/assets": "^1@dev",

I think it's safe to assume it will always be there.

@maxime-rainville
Copy link
Contributor Author

Just had a quick look to see if it would possible to decouple framework from assets. There's references to assets pretty much everywhere, even in things that you wouldn't think have anything to do with managing files.

@michalkleiner
Copy link
Contributor

So... is extracting assets into its own module a good thing? Should it be again a part of the framework?

@maxime-rainville
Copy link
Contributor Author

In the long run, I think there's some value, if only to provide a way to precisely target which version of assets you want to use.

We probably should avoid adding more inter-dependencies ... although I don't think this PR is the straw that will break this camel's back.

@chillu
Copy link
Member

chillu commented Apr 5, 2019

🐪

@chillu chillu merged commit afb3c82 into silverstripe:4 Apr 5, 2019
@chillu chillu deleted the pulls/4/get-installer-to-self-create-assets-folder branch April 5, 2019 01:41
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.

6 participants