Added merge_dicts Jinja2 filter #7629

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
6 participants
@AndrewPashkin
Contributor

AndrewPashkin commented Jun 1, 2014

Added a new simple filter for merging dicts manually and not in-place.
Ther was the discussion on mailing list.

ps.
Should I have to add the docs section?

@jirutka

This comment has been minimized.

Show comment
Hide comment
@jirutka

jirutka Jun 2, 2014

Contributor

+1

Contributor

jirutka commented Jun 2, 2014

+1

@jimi-c jimi-c added P3 labels Jun 3, 2014

@jimi-c

This comment has been minimized.

Show comment
Hide comment
@jimi-c

jimi-c Jun 3, 2014

Member

Yes, this also needs to be added to the docs. Be sure to include the version added information (see other entries there for examples). Thanks!

Member

jimi-c commented Jun 3, 2014

Yes, this also needs to be added to the docs. Be sure to include the version added information (see other entries there for examples). Thanks!

@AndrewPashkin

This comment has been minimized.

Show comment
Hide comment
@AndrewPashkin

AndrewPashkin Jun 3, 2014

Contributor

How can I know in which version this feature will be released?

Contributor

AndrewPashkin commented Jun 3, 2014

How can I know in which version this feature will be released?

@sivel

This comment has been minimized.

Show comment
Hide comment
@sivel

sivel Jun 3, 2014

Member

Assume for the time being that it is 1.7. If it doesn't make it into 1.7 you will just need to bump it at a later date.

Member

sivel commented Jun 3, 2014

Assume for the time being that it is 1.7. If it doesn't make it into 1.7 you will just need to bump it at a later date.

@jimi-c

This comment has been minimized.

Show comment
Hide comment
@jimi-c

jimi-c Jun 3, 2014

Member

Or we will fix it when we merge it in.

Member

jimi-c commented Jun 3, 2014

Or we will fix it when we merge it in.

@AndrewPashkin

This comment has been minimized.

Show comment
Hide comment
@AndrewPashkin

AndrewPashkin Jun 4, 2014

Contributor

I've added 1.7

Contributor

AndrewPashkin commented Jun 4, 2014

I've added 1.7

@darkk

This comment has been minimized.

Show comment
Hide comment
@darkk

darkk Jun 19, 2014

Contributor

Is the merge intentionally shallow?

hash_behaviour=merge produced deep merge, so POLA implies that filter merge_dicts should provide deep merge too instead of shallow one.

Actually, I've already implemented merge_hash filter, so I wonder what is the correct way to contribute it:

  • create another pull request with merge_dict filter
  • rename my merge_hash to merge_deep, but it leads to two different merge filters in ansible that may be confusing
Contributor

darkk commented Jun 19, 2014

Is the merge intentionally shallow?

hash_behaviour=merge produced deep merge, so POLA implies that filter merge_dicts should provide deep merge too instead of shallow one.

Actually, I've already implemented merge_hash filter, so I wonder what is the correct way to contribute it:

  • create another pull request with merge_dict filter
  • rename my merge_hash to merge_deep, but it leads to two different merge filters in ansible that may be confusing
@jirutka

This comment has been minimized.

Show comment
Hide comment
@jirutka

jirutka Jun 20, 2014

Contributor

@darkk The best solution would be IMHO merge these two filters and use a parameter to choose between deep (default), or shallow merge. Maybe something like foo | merge_hash(deep=false) for shallow merge.

Contributor

jirutka commented Jun 20, 2014

@darkk The best solution would be IMHO merge these two filters and use a parameter to choose between deep (default), or shallow merge. Maybe something like foo | merge_hash(deep=false) for shallow merge.

@ansibot ansibot added feature and removed feature_pull_request labels Mar 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment