Reduce boilerplate: reconsider build-assets script

[stevector]

Here is a break-out issue from https://github.com/pantheon-systems/example-drops-8-composer/issues/61#issuecomment-315160708

In that comment @greg-1-anderson questioned if build-assets, which wraps composer install is needed. Could we instead just call composer install and expect project to have hooked into that process?

I think we should. Our composer.json already has additional commands running in post-install-cmd

    "post-install-cmd": [
      "@drupal-scaffold",
      "DrupalProject\\composer\\ScriptHandler::createRequiredFiles"
    ],

I suggest we deprecate build-assets and only run composer install. For pre-existing sites, the Build Plugin will need to check if build-assets exists.

[greg-1-anderson]

I didn't actually recommend removing build-assets in the referenced issue; I suggested we might need something other than a composer script to build assets, and I suggested that perhaps the tool should assume composer install should always be executed, so that it does not need to be explicitly called in build-assets.

I'm reluctant to make build-assets part of the Composer install step. What about sites that need to use nodejs or ruby tools in order to build their assets? I think there might be times that we want to run composer install or composer update without building the assets. Composer lock update is one clear use-case for the later, and I'm not sure that we should build assets in composer install but not composer update (users expect the later does the former). Even looking only at composer install, we might want to install without the build assets to do code sniffing, e.g. as the Example WordPress Composer circle 2.0 tests currently do.

[stevector]

I didn't actually recommend removing build-assets in the referenced issue;

Sorry! I misunderstood.

I suggested we might need something other than a composer script to build assets

Do you have something in mind?

we might want to install without the build assets to do code sniffing

Yeah, that probably makes doing build-assets in composer-install a non-starter.

My thinking here was that composer install is that command for getting the code needed to run your site. And if a site needs to compile non-PHP dependencies to get a workable codebase, then so be it. But for the slowness that could result I don't think we can impose that decision.

[greg-1-anderson]

Presently, I think that a composer script is the right way to handle this. I am seeing other people in the PHP / Composer community move in this direction. For example, I have seen composer build + composer test being recommended as a standard workflow that folks should be able to run immediately after a git checkout to run tests for the project.

I go back and forth about wanting to rename build-assets to simply build. I shied away from this in case someone wanted to have a more heavyweight operation for build that might include build-assets as just one step. However, I'm not sure what else might happen in a build step. Would it be appropriate to have a Composer script that installed nodejs &/or Ruby components, if needed?

I'm not really sure what's best here vis-a-vis script naming, but I think that using a Composer script to build the assets is still a good idea. The example wp composer project also adds a Composer script for doing code sniffing, which also seems like a good idea to me.

[stevector]

Presently, I think that a composer script is the right way to handle this.

Works for me. I had opened this issue because I thought there was a clear action to take. I now don't think there is. How about we mark this as postponed?

Would it be appropriate to have a Composer script that installed nodejs &/or Ruby components, if needed?

Maybe. Are you thinking something like build-non-php-dependencies which could then be called from build-assets? I could see that being helpful but I don't want add more complexity without knowing that someone actually needs it.

Since this repo is an example meant to be modified, we could do something like

    "build-assets": [
      "@prepare-for-pantheon",
      "composer install --optimize-autoloader",
      "echo If your site needs to install more dependencies with Ruby, NPM, Yarn, or anything else, add commands like 'npm install' here in this 'build-assets' section of composer.json"
    ],

[greg-1-anderson]

I agree that we should not stub out nodejs / ruby component installation in our base template projects.

I don't know that we need an echo in build-assets, as long as these instructions are documented somewhere.