Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Make environment-specific server configurations fully override the default one. #138

Closed
wants to merge 3 commits into from

Conversation

sangaline
Copy link
Contributor

This would change the behavior of the configuration merging to allow for shorter lists of servers to completely override a longer set of defaults. Further discussion can be found in #137.

@timkelty timkelty closed this in 5501d90 Nov 13, 2016
@timkelty
Copy link
Member

@sangaline Thanks for the PR! - I ended up grabbing your test, but I decided to just use _.assign instead of making an exception for servers.

@sangaline
Copy link
Contributor Author

Thanks!

timkelty pushed a commit that referenced this pull request Nov 22, 2016
timkelty pushed a commit that referenced this pull request Nov 22, 2016
@timkelty
Copy link
Member

@sangaline It seems a lot of people were actually relying on this type of merge, so it really should have been published as a breaking change.

So, I reverted the change with a patch (1.5.2), and re-released it as (2.0.0).

@twalling
Copy link
Contributor

twalling commented Mar 6, 2017

I know this is closed but I'm a little surprised at the drastic change in how configs were merged. This requires existing users to rewrite configs if they want to upgrade. This just bit me for a little while and I'd suggest an addition to the README near the top calling out the change from _.merge to _.assign.

@timkelty
Copy link
Member

timkelty commented Mar 6, 2017

@twalling sorry about that.
For that reason it was published as a major version change, but yes - a note in the Readme is certainly warranted.

It's on my list to add, also happy to accept a PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants