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

config:import changed to support config.storage.sync service for default instance #3902

Conversation

jenswegar
Copy link
Contributor

Reason for this pull request:

I'm trying to use the drupal/config_ignore module, which in turn relies on drupal/config_filter module to maintain a separate local dev/staging/prod configuration. Config_ignore has changed how it works internally and relies on config_filter to provide the filtering mechanism. config_filter in turn decorates the config.storage.staging/sync service in order to inject it's own FilteredStorage class to handle the filtering. But the current Drupal console class src/Command/Config/ImportCommand.php creates a FileStorage instance directly inside it's execute command, instead of relying on the instance returned from \Drupal::service('config.storage.sync'). Thus the decorated class is never made available to the import command and the import filtering does not work.

This pull request changes the ImportCommand so that in the default case (no --directory option provided) it uses the FileStorage instance from the service container instead of creating it's own local instance. AFAIK this does not break anything, but adds the possibility to override the filestorage by using decorators, like what the config_filter module does.

@jmolivas jmolivas added this to the 1.9.0 milestone Jun 28, 2018
@enzolutions
Copy link
Contributor

Thanks @jenswegar for your contribution.

@enzolutions enzolutions merged commit bfce216 into hechoendrupal:master May 21, 2019
@jenswegar jenswegar deleted the feature/config-import-storage-service-support branch July 8, 2019 09:34
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