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

Add the overwrite_arrays option #103

Closed
wants to merge 1 commit into from

Conversation

dtaniwaki
Copy link
Contributor

I think we should be able to diable merging arrays.

e.g. Let's say you have the default settings like below.

config/settings.yml

email:
  recipients:
    - foo1@example.com
    - foo2@example.com

And for production settings.

config/settings/production.yml

email:
  recipients:
    - bar@example.com

RailsConfig merges the settings like below.

email:
  recipients:
    - foo1@example.com
    - foo2@example.com
    - bar@example.com

However, I don't want to send email to foo1@example.com and foo2@example.com in production. Currently, we don't have any method to achieve this goal. So I just added an option to disable merging arrays.

What do you think this idea?

Review on Reviewable

@fredwu
Copy link
Member

fredwu commented Jun 27, 2015

Hi @dtaniwaki, this is a great idea! Implementation wise, PR #110 has just been merged in - we now use the gem version of deep_merge, is this still achievable without changing deep_merge's source code?

@dtaniwaki
Copy link
Contributor Author

@fredwu
Let me try it!

@hnakamur
Copy link

@dtaniwaki Thanks for trying to update your pull request.

Maybe you can use test cases added by me. I updated my branch to rewrite test cases with rspec expect: master...hnakamur:overwrite_arrays

Note I have not updated my implementation. I leave it up to you.

Fryguy added a commit to Fryguy/deep_merge that referenced this pull request Apr 1, 2016
This is an extraction from
rubyconfig/config#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

Paired with @carbonin <https://github.com/carbonin> on this.
Fryguy added a commit to Fryguy/config that referenced this pull request Apr 1, 2016
This is an extraction from
rubyconfig#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

This depends on danielsdeleo/deep_merge#21

Paired with @carbonin <https://github.com/carbonin> on this.
Fryguy added a commit to Fryguy/deep_merge that referenced this pull request Apr 1, 2016
This is an extraction from
rubyconfig/config#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

Paired with @carbonin <https://github.com/carbonin> on this.
Fryguy added a commit to Fryguy/config that referenced this pull request Apr 1, 2016
This is an extraction from
rubyconfig#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

This depends on danielsdeleo/deep_merge#21

Paired with @carbonin <https://github.com/carbonin> on this.
Fryguy added a commit to Fryguy/deep_merge that referenced this pull request Apr 1, 2016
This is an extraction from
rubyconfig/config#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

Paired with @carbonin <https://github.com/carbonin> on this.
@pkuczynski pkuczynski added this to the 1.x overwrite arrays milestone May 12, 2016
Fryguy added a commit to Fryguy/deep_merge that referenced this pull request May 20, 2016
This is an extraction from
rubyconfig/config#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

Paired with @carbonin <https://github.com/carbonin> on this.
vinceve pushed a commit to vinceve/config that referenced this pull request Jul 27, 2016
This is an extraction from
rubyconfig#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

This depends on danielsdeleo/deep_merge#21

Paired with @carbonin <https://github.com/carbonin> on this.
Fryguy added a commit to Fryguy/config that referenced this pull request Aug 1, 2016
This is an extraction from
rubyconfig#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

This depends on danielsdeleo/deep_merge#21

Paired with @carbonin <https://github.com/carbonin> on this.
@pkuczynski
Copy link
Member

Fixed in #137 using newer deep_merge gem

@pkuczynski pkuczynski closed this Sep 13, 2016
devs-cloud pushed a commit to devs-cloud/ruby-config that referenced this pull request Dec 20, 2019
This is an extraction from
rubyconfig/config#103 where @dtaniwaki had
added this option, but had done it prior to the deep_merge extraction.

This depends on danielsdeleo/deep_merge#21

Paired with @carbonin <https://github.com/carbonin> on this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants