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

Initial take on adding Drupal core composer install for >= 8.1.0 #277

Closed
wants to merge 2 commits into from

Conversation

grayside
Copy link
Contributor

This PR checks to see if the version of Drupal is >= 8.1, and if so it adds an extra composer:install operation. The theory of the require statements is to make sure it does not operate unless drushmake and scaffold already ran. This ensures the codebase with it's composer.json is already present, and that the custom modules and libraries from the codebase have been symlinked in for any composer autoloader magic.

I suppose an alternative to adding the semver check would be looking for composer.json and !vendor.

The build process is currently incomplete, now that Drupal 8.1 does not ship the vendor directory pre-installed.

@arithmetric @jhedstrom

Warning: Untested changeset.

@grayside grayside added the bug label Apr 23, 2016
@grayside
Copy link
Contributor Author

grayside commented May 3, 2016

This issue is is slightly mitigated because packaged versions of Drupal core on Drupal.org already include the vendor directory. However, other forks of Drupal Core likely do not use this, nor do git checkouts.

The next hurdle to this issue is that our Drupal version detection is dependent on drush status, and that command is dependent on bootstrapping Drupal which requires the composer autoloader. Since problems relying on drush in this way have come up before, maybe we should adjust lib/drupal.js?

@arithmetric
Copy link
Collaborator

@grayside Now that #284 has been merged, I believe the goals of this PR have been accomplished. Do you see any follow ups needed or can this be closed?

@grayside grayside closed this Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants