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

Use recursive config patching #628

Closed

Conversation

mkfrey
Copy link
Contributor

@mkfrey mkfrey commented Oct 15, 2019

  • More flexible patching
  • Better readable code

* More flexible patching
* Better readable code
@mkfrey mkfrey force-pushed the develop-v3-recursive-patching branch from 28aa4fd to b13610d Compare October 15, 2019 11:44
@mkfrey mkfrey mentioned this pull request Oct 15, 2019
@mkfrey
Copy link
Contributor Author

mkfrey commented Oct 15, 2019

This PR will make the config patching algorithm recursive.
This results in the following benefits:

  • The patching code is easier to read
  • The storage footprint is lower
  • All future changes to the config structures and config depth should be covered

The patching has the following behaviour:

  • If an element in the patch does not exist in the current config, it will be set to the patch element.
  • If an element in the patch is not an object or the element in the current config is not an object, it will be set to the patch element.
  • If an element in the patch is an object and the element in the current config is an object all described steps will be repated with these two objects.

@mkfrey
Copy link
Contributor Author

mkfrey commented Jan 2, 2020

Please don't merge yet. With the password being optional after merging #647, I can rewrite the patching to support the removal of configuration values by setting them to null in the patch.

@stritti stritti closed this Jan 3, 2020
@stritti
Copy link
Collaborator

stritti commented Jan 3, 2020

@codefrey could you rebase to branch develop please?
Renaming the branches closed the PR. Sorry.

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

Successfully merging this pull request may close these issues.

2 participants