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

Connects to #965: Add --config-dir option to Drush site-install command. #966

Closed
wants to merge 1 commit into from
Closed

Connects to #965: Add --config-dir option to Drush site-install command. #966

wants to merge 1 commit into from

Conversation

geerlingguy
Copy link
Contributor

Relates to Issue #965.

Changes proposed:

  • Add --config-dir option to Drush site-install command.
  • Empower sites using full CMI exports (either via core CMI or with Config Split) to be installed in CI or other environments (otherwise the install fails on UUID issues).

Note: This should be tested on sites that are currently not using full CMI export/import to make sure nothing is broken in the build in that situation.

@grasmash grasmash changed the title Issue #965: Add --config-dir option to Drush site-install command. Fixes #965: Add --config-dir option to Drush site-install command. Jan 12, 2017
@grasmash
Copy link
Contributor

grasmash commented Jan 12, 2017

@geerlingguy just FYI, you need to use one of the following keywords to "connect" an issue in GitHub to another:

close
closes
closed
fix
fixes
fixed
resolve
resolves
resolved

E.g., "Fixes #965" rather than "Issue #965"

See https://help.github.com/articles/closing-issues-via-commit-messages/

@geerlingguy
Copy link
Contributor Author

geerlingguy commented Jan 12, 2017

@grasmash This wouldn't close that issue though... it's just related. E.g. the first little bit that would go a ways towards helping two use cases. So in this case I still hope the PR can get merged, but I don't want it to close #965.

@geerlingguy
Copy link
Contributor Author

Error is:

Import the listed configuration changes? (y/n): y
Drupal\Core\Config\ConfigImporterException: There were errors        [error]
validating the config synchronization. in
Drupal\Core\Config\ConfigImporter->validate() (line 728 of
/home/travis/build/acquia/blt-project/docroot/core/lib/Drupal/Core/Config/ConfigImporter.php).
The import failed due for the following reasons:                     [error]
This import is empty and if applied would delete all of your
configuration, so has been rejected.

Hmm... so our automated tests would only work if you have at least a tiny bit of config exported.

@grasmash
Copy link
Contributor

@geerlingguy Re:keywords. Fair enough. In that case, let's use Waffle.io's keywords so that issues are visually connected on our Kanban board:

connect to
connects to
connected to
connect
connects
connected

@geerlingguy
Copy link
Contributor Author

We have a kanban board??

@geerlingguy
Copy link
Contributor Author

@grasmash - I updated the commit message at least—haven't looked into making Travis happy.

@geerlingguy geerlingguy changed the title Fixes #965: Add --config-dir option to Drush site-install command. Connects to #965: Add --config-dir option to Drush site-install command. Jan 12, 2017
@grasmash grasmash added the Enhancement A feature or feature request label Feb 6, 2017
@grasmash
Copy link
Contributor

grasmash commented Feb 6, 2017

@geerlingguy:

The import failed due for the following reasons: [error]
This import is empty and if applied would delete all of your
configuration, so has been rejected.

@geerlingguy
Copy link
Contributor Author

@grasmash - Yeah... not sure how to deal with that—this is necessary after you've done your first config dump, but it doesn't work until that's been done. Chicken and egg issue.

Maybe we can have a flag/if/else thing that can say "enable this in the build" and have it off by default?

@grasmash
Copy link
Contributor

I think this is basically a duplicate of #884

@geerlingguy
Copy link
Contributor Author

@grasmash - Yeah, basically. It would be nice if we had a flag that you could set to enable this option once the project is up and running—and only if you choose to use full CMI (and not --partial).

@geerlingguy
Copy link
Contributor Author

That would also allow us to default to the current behavior, which can pass tests here in Travis since there isn't any config to kick off a brand new site/project.

grasmash added a commit to grasmash/bolt that referenced this pull request Feb 13, 2017
…uration directory and partial flag to be configurable.
grasmash added a commit that referenced this pull request Feb 13, 2017
@geerlingguy
Copy link
Contributor Author

Closing since #1080 got in.

@geerlingguy geerlingguy deleted the config-split branch March 21, 2017 18:58
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.

2 participants