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

Extending an array config only overrides specific elements of the array #31

Open
danepowell opened this issue Jun 4, 2019 · 1 comment

Comments

@danepowell
Copy link

danepowell commented Jun 4, 2019

I'm opening this in reference to a downstream issue with BLT (which uses consolidation/config): acquia/blt#3697

BLT works by providing a default set of configuration, and then calling ConfigProcessor::extend() to override any values with user-provided variants (via blt.yml and other sources).

As you can see from that issue, the problem this user is facing is that when they want to override an array, they have to override every element of that array individually. I think it would make more sense that if you override any part of an array, it resets the entire array.

For instance, if the array is this:

sync:
  commands:
    - op1
    - op2

We extend it as:

sync:
  commands:
    - opfoo

We expect the final overridden config to be:

sync:
  commands:
    - opfoo

But we actually get:

sync:
 commands:
   - opfoo
   - op2

Is this expected behavior? Is there some other workaround that I'm missing?

@danepowell danepowell changed the title Extending an array config only overrides specific members of the array Extending an array config only overrides specific elements of the array Jun 4, 2019
@greg-1-anderson
Copy link
Member

Unordered lists are a weak spot in configuration override. Configuration is combined using @grasmash 's ArrayUtil::mergeRecursiveDistinct. I think that whether you want to replace or merge array elements might depend on the context.

If we did change the behavior here, it would have to either be via an option (a method call a client could make), or be part of a new major release, as it would be a backwards-incompatible change.

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

No branches or pull requests

2 participants