[WIP] Add knockout prefix for removed array elements #7656
Conversation
|
@miq-bot add_label wip, bug, core |
|
@carbonin Cannot apply the following label because they are not recognized: wip bug core |
|
@Fryguy At this point the correct final hash is visible in I suspect this is a separate issue. |
| @@ -19,6 +19,8 @@ def self.diff(h1, h2) | |||
| child = | |||
| if v1.kind_of?(Hash) && v2.kind_of?(Hash) | |||
| diff(v1, v2) | |||
| elsif v1.kind_of?(Array) && v2.kind_of?(Array) | |||
| v2 + (v1 - v2).map { |x| "#{KNOCKOUT_STRING}#{x}" } | |||
There was a problem hiding this comment.
I don't think this should live in the HashDiffer as that class should not be aware of the config gem (it's generic diffing code)...let's discuss today if you have time.
The config gem merges array values by default so we need to specify a prefix to elements that we want to explicity remove from the template. NOTE: We will need a different solution for non-string values
carbonin
force-pushed
the
add_knockout_to_config_settings_diff
branch
from
0faeb68
to
643eb34
Compare
|
Checked commit carbonin@643eb34 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1 |
|
<github_pr_commenter_batch />Some comments on commit carbonin@643eb34 |
|
Closing in favor of #7670 |
The ability to override Arrays has been made into a PR upstream at rubyconfig/config#137 and danielsdeleo/deep_merge#21 . However, they are not yet merged. When they are merged and released, we can switch back to a released version. Supercedes #7656 Paired on this with @carbonin
The ability to override Arrays has been made into a PR upstream at rubyconfig/config#137 and danielsdeleo/deep_merge#21 . However, they are not yet merged. When they are merged and released, we can switch back to a released version. Supercedes ManageIQ#7656 Paired on this with @carbonin
By default the config gem merges array values in the various config sources.
This means that removing elements from an array value will never take effect.
The knockout prefix tells the gem that the matching elements in other config sources should be removed.
@Fryguy please review