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

WIP - Support for WordPress example repo #39

Closed
wants to merge 5 commits into from

Conversation

stevector
Copy link
Collaborator

Here is a work-in-progress PR to replace #9

This PR should not be merged until

@greg-1-anderson
Copy link
Member

We also need to make an Empty Upstream for WordPress (and eventually Drupal 7 as well).

// or 'Drupal 7' or 'WordPress'
// return 'Drupal 8';
if (file_exists($siteDir . '/wp-cli.yml')) {
return 'WordPress';
Copy link
Member

Choose a reason for hiding this comment

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

I created a new empty upstream w/ the framework set to 'wordpress' named 'Empty WordPress'

return 'WordPress';
}
else {
// This upstream works for Drupal 8 and Drupal 7.
Copy link
Member

Choose a reason for hiding this comment

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

Actually, 'Empty Upstream' only works for Drupal 8. I made another upstream named 'Empty 7' for Drupal 7. Maybe test for "$siteDir/web/misc/drupal.js" as an indicator that the site is Drupal 7.

return 'Empty Upstream';
// or 'Drupal 7' or 'WordPress'
// return 'Drupal 8';
if (file_exists($siteDir . '/wp-cli.yml')) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe test for "$siteDir/web/wp-admin/index.php"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Composer-based way of doing WordPress moves wp-admin too. In our example it is web/wp/wp-admin. I don't think we can assume that. I'm leaning toward web/wp-config.php. @ataylorme, do you have thoughts on what file would be the best to check for to determine if a repo is WordPress?

Copy link
Member

Choose a reason for hiding this comment

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

web/wp-config.php sounds like a good option to me. Worst case, we could test two locations (e.g. web/wp/wp-admin/index.php and web/wp-admin/index.php). I don't think it's a problem to put a limitation on where WordPress can be relocated to (e.g. wp or nothing).

If testing wp-config.php sidesteps this issue, even better.

return 'Empty Upstream';
// or 'Drupal 7' or 'WordPress'
// return 'Drupal 8';
if (file_exists($siteDir . '/web/wp-config.php')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@stevector and @greg-1-anderson WordPress will first look for wp-config.php wherever core is. If WordPress core is in a subdirectory, say wp, then web/wp/wp-config.php will take precedence over web/wp-config.php if it exists.

That tripped me up when first implementing WordPress is a subdirectory via Composer as the Pantheon WordPress upstream includes wp-config.php. When installing core from our upstream repo into a subdirectory web/wp/wp-config.php must be deleted.

All that being said someone could have web/wp/wp-config.php, or wherever they have installed WordPress core, without web/wp-config.php and have a properly working site. I think it's okay for us to be opinionated about using wp for the directory name as well as having wp-config.php live in the we docroot.

Copy link
Member

Choose a reason for hiding this comment

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

A variant of this line was already committed as part of a separate PR. We could check for both web/wp-config.php and web/wp/wp-config.php, or test some file that is always in the docroot, even when core is relocated, if such a candidate exists.

Copy link
Member

Choose a reason for hiding this comment

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

Let's continue in #60.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use something like find -maxdepth 2 -type f -name 'wp-config.php rather than hard-coding directories or guessing what subdirectory will have WordPress core.

@stevector
Copy link
Collaborator Author

#60 Is a switch to Circle 2 that tests D8 and WordPress.

@greg-1-anderson
Copy link
Member

Closing in favor of #60.

@stevector stevector deleted the wordpress-build-plugin branch August 30, 2017 01:56
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