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

Add post-config-import hook. #1108

Closed
wants to merge 2 commits into from
Closed

Add post-config-import hook. #1108

wants to merge 2 commits into from

Conversation

bobbygryzynger
Copy link
Contributor

Changes proposed:

  • Adds a post configuration import hook.

@grasmash grasmash added the Enhancement A feature or feature request label Feb 16, 2017
@danepowell
Copy link
Contributor

Makes sense. I'm curious how it would be used though.

@bobbygryzynger
Copy link
Contributor Author

bobbygryzynger commented Feb 16, 2017

@danepowell - my team would like to use it to toggle local modules.

For instance, when developing sites we use devel (who doesn't).

When we run

blt setup:toggle-modules -Dmultisite.name=<site> -Denvironment=local

this enables devel for us, but then if we run

setup:config-import -Dmultisite.name=<site>

this will fail because our committed code doesn't list devel as enabled in core.extension.yml.

If we had a post-config-import hook we could attach the module toggling to setup:config-import allowing us a bit more consistency in our internal PRs ensuring our devs only toggle development modules after production configuration has been imported and save us all a few key strokes.

Has your team run into this issue? I'd be interested to hear how you've been solving it if so.

@danepowell
Copy link
Contributor

There seem to be two general approaches to CM right now.

The first is to use Features or partial config imports, which implies not using core.extensions.yml and instead leaving the enabling / disabling of extensions to update hooks and blt setup:toggle-modules. I personally don't use core.extensions.yml because back in the day there were some pretty nasty bugs related to it (installing corrupt or non-existent table schemas, etc), and I'm not 100% confident they've been resolved.

The second approach is to do full config imports, but use config_split to support varying configuration (including installed modules) by environment. As an example, see @geerlingguy's blog post (especially the first user comment)

@bobbygryzynger
Copy link
Contributor Author

bobbygryzynger commented Feb 16, 2017

Thanks @danepowell, these alternative approaches are definitely something our team needs to evaluate.

@grasmash
Copy link
Contributor

Yeah I'd also like to see a strong use case presented for this. I'm wary of introducing too many new target-hooks.

@bobbygryzynger
Copy link
Contributor Author

bobbygryzynger commented Feb 17, 2017

@grasmash I agree, feel free to close this, really what is needed is a solution to #1113, which I should have a PR for shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants