Use new Drupal Finder API. #673
Conversation
| @@ -44,7 +44,7 @@ public static function createRequiredFiles(Event $event) { | |||
| require_once $drupalRoot . '/core/includes/install.inc'; | |||
| new Settings([]); | |||
| $settings['settings']['config_sync_directory'] = (object) [ | |||
| 'value' => Path::makeRelative($drupalFinder->getComposerRoot() . '/config/sync', $drupalRoot), | |||
| 'value' => '../config/sync', | |||
There was a problem hiding this comment.
With new Drupal Finder version 1.3, $drupalFinder->getComposerRoot() return empty string not same the old one.
More info webflo/drupal-finder#72
I think we can safety hard code relative path ../config/sync at here. Drupal can handle relative path to Drupal root itself
There was a problem hiding this comment.
@tannguyen04 – I was curious if this still works when you put a nested folder for extra.drupal-scaffold.locations.web-root (e.g. web/test/) but this is not how extra.drupal-scaffold.locations.web-root is supposed to work. You can not have nested directories at this point. So ../config/sync should be fine.
There was a problem hiding this comment.
This also fixed the tests not passing in #650. 👌🏻
There was a problem hiding this comment.
Sound like waiting for a response on webflo/drupal-finder#72 or diving into that further might be the best approach here?
There was a problem hiding this comment.
"safety hard code relative path ../config/sync" is breaking our install and update process because composer root, drupal root and config root are at the same level.
I will manually override the dir in our project and will wait until the webflo-issue is fixed.
There was a problem hiding this comment.
Ahhh, of course you can have . as web root and then it breaks. 💡
There was a problem hiding this comment.
@sascha-krug - would you be so kind and create a follow-up issue for it, please? I don't know when I've time at the computer again.
AlexSkrypnyk
added
the
PR: Needs review
No description provided.