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

Enhance defaults.merge function. #44850

Merged
merged 7 commits into from
Dec 9, 2017
Merged

Enhance defaults.merge function. #44850

merged 7 commits into from
Dec 9, 2017

Conversation

aabouzaid
Copy link
Contributor

@aabouzaid aabouzaid commented Dec 6, 2017

What does this PR do?

In defaults.merge function:

  • Add merge_lists option.
  • Add in_place option.
  • Write unit tests.

Previous Behavior

Lists were not merged, and no unit test for merge function.

New Behavior

  • Allow to merge lists.
  • Allow to keep original dicts untouched and create new dict.
  • Unit test.

Tests written?

Yes

Commits signed with GPG?

Not yet

@rallytime
Copy link
Contributor

@aabouzaid
Copy link
Contributor Author

aabouzaid commented Dec 6, 2017

@rallytime Akh, a typo 🤕
Fixed :-)

@aabouzaid
Copy link
Contributor Author

I also added an arg to don't copy in place, so it keeps the orig dicts untouched.
The original behavior still the same (in_place=True by default).

@garethgreenaway
Copy link
Contributor

@aabouzaid Looks like there are still a couple more lint errors. Would you mind taking care of those?

@aabouzaid
Copy link
Contributor Author

@garethgreenaway Fixed :-)

@garethgreenaway
Copy link
Contributor

@aabouzaid Awesome! Thanks! Just waiting for automated tests to finish then we should be good to go.

@garethgreenaway garethgreenaway merged commit 374d1ef into saltstack:develop Dec 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants