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

Added new config:import:list and config:export:list commands. Fix no return on configImport method for config:import:single command #2965

Merged
merged 6 commits into from
Dec 17, 2016

Conversation

bpresles
Copy link
Contributor

Added 2 commands:

config:import:list

Takes a YAML file with list of config names to import. Allow to import restricted list of config names easily (rather than doing multiple config:import:single for each config name)

config:export:list

Takes a YAML file with list of config names to export. Allow to export restricted list of config names easily (rather than doing multiple config:export:single for each config name)

Also includes a fix in configImport method of config:import:single command that doesn't return boolean on success (making the success message not displayed).

Bertrand PRESLES added 2 commits November 21, 2016 17:40
@jmolivas
Copy link
Member

@bpresles I like this idea but since we have config:import:single and config:export:single how about trying to merge functionality.

@bpresles
Copy link
Contributor Author

@jmolivas
Yes why not. Would you prefer to merge this in existing config:import:single and config:export:single commands by adding for example a --list [configs list YAML file]?
Or create new commands name offering both functionality (in that case which names would you suggest?)?

Once you tell me you preference I'll work on it and create a new pull request (it will be cleaner).

@jmolivas
Copy link
Member

jmolivas commented Nov 24, 2016

@bpresles I think we can do something like this to merge with current config:import:single and config:export:single commands.

My first idea or recommended approach is to change ExportSingleCommand
https://github.com/hechoendrupal/DrupalConsole/blob/master/src/Command/Config/ExportSingleCommand.php#L62-L66

  • Make config-name an option
  • Make config-name an array InputOption::VALUE_IS_ARRAY
  • Create an example YAML file to use it via a chain command
  • Implement code similar to what you have on this PR to process multiple configurations.
commands:
  - command: 'config:export:single'
    options:
        config-name: 
          - core.extension
          - contact.settings

Add documentation how to use it:

drupal chain --file=path/to/multile-config.yml

NOTE: A chain command can read a YAML file and execute one or multiple commands previously defined on that YAML file.

Examples:

@jmolivas
Copy link
Member

jmolivas commented Dec 2, 2016

@bpresles still interested to make this happen?

@bpresles
Copy link
Contributor Author

bpresles commented Dec 2, 2016

@jmolivas Yes sure. I didn't have much time these days to work on it. I'll look at it this week end.

@bpresles
Copy link
Contributor Author

bpresles commented Dec 7, 2016

@jmolivas Just to keep you informed, I didn't have much time this week end finally. I hope to be able to work on it and finish it for the end of the week at most.

@bpresles bpresles force-pushed the master branch 2 times, most recently from 4402f05 to a23202f Compare December 11, 2016 21:30
Bertrand PRESLES added 2 commits December 11, 2016 22:32
…e configs with the config:import:single and config:export:single commands
@bpresles
Copy link
Contributor Author

@jmolivas
I've done the merge. To avoid any compatibility issue with previous version, I've kept the original arguments (name and file for import:single, config-name for export:single) adding a new "config-names" option that allows the use of these commands with chain command and a list of config names.

@jmolivas
Copy link
Member

jmolivas commented Dec 17, 2016

@bpresles Thanks for refactoring I have some comments:

  • Lets use only one argument make config-name have the functionality of config-names and remove config-names.

  • Remove the files export-config-list-sample.yml and import-config-list-sample.yml
    We can add those as examples to the docs instead.

@jmolivas jmolivas added this to the 1.0.0-rc12 milestone Dec 17, 2016
@jmolivas
Copy link
Member

I saw some other old issues to fix like, argument names ons is called config-name the other only name

@jmolivas
Copy link
Member

I will go ahead and have this merged and I will send a PR with mentioned fixes.

@jmolivas jmolivas merged commit 259019c into hechoendrupal:master Dec 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants