Added features bundle argument. #479
Conversation
| # Configuration management options, such as core CM or features. | ||
| cm: | ||
| features: | ||
| bundle: blted8 |
There was a problem hiding this comment.
Looks like this is now required, is that correct? What will this do to existing projects that don't have this defined?
Maybe we should just use ${project.machine_name} rather than creating a new variable. Is there value in being able to customize it?
| @@ -272,7 +272,9 @@ | |||
| <param>drush</param> | |||
| </drush> | |||
| <!-- Revert all features. --> | |||
| <drush command="fra" assume="yes" alias="${drush.alias}"/> | |||
| <drush command="fra" assume="yes" alias="${drush.alias}"> | |||
| <option name="bundle">${cm.features.bundle}</option> | |||
There was a problem hiding this comment.
Can this be optional? Will it fail if the cm.features.bundle is unset?
|
Features itself doesn't require a bundle argument, but you're right, Phing would fail if it was undefined. I can't imagine anyone would want to run features reverts without a bundle argument (now that bundle arguments are available), so I added a conditional based on the bundle being defined. Some folks will probably be pretty happy that they can now choose whether to revert features or not. |
| @@ -58,6 +58,11 @@ target-hooks: | |||
| dir: ${docroot}/profiles/contrib/lightning | |||
| command: npm run install-libraries | |||
|
|
|||
| # Configuration management options, such as core CM or features. | |||
| # cm: | |||
There was a problem hiding this comment.
I'm a little hesitant to introduce a new top level array into this file, unless we really think we're going to build out other options.
What if we just made the assumption that we'd use the machine name?
There was a problem hiding this comment.
I thought about that, but it won't work for a lot of existing projects that already have different machine and bundle names (including mine). It also wouldn't let you specifically disable features integration.
Note that if you provide a non-existent bundle name, the features revert will still run but will throw a warning.
If you don't want to introduce a new top-level config, we could just put this under "project" for now until we decide to expand CM support.
Currently, if you run fra (features-import-all) on a Lightning-based install, you'll get all of Lightning's default config, which (a) is annoying, because it clobbers your own config if you've overridden Lightning, and (b) goes against the Lightning pattern of profile inheritance, which is to push configuration updates via hook_update_n rather than relying on features reverts.
I organized the bundle argument into a separate
cmroot-level configuration option, because I think we might want to start being a little more flexible about CM options for projects. For instance, a lot of projects might not use features or core CM at all and might want options to enable them individually. In the future, we could read config values such ascm.featuresorcm.coreto decide whether to runfraorconfig-import, respectively.