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

Fix #443 object merge with same value on same level keys #444

Merged
merged 1 commit into from Oct 8, 2020
Merged

Conversation

rochacbruno
Copy link
Member

This solution is a temporary solution as it solves current
problem, but there is still the case for None values.

The best solution for this case would be wrapping all the values
on assignment and give it a full path signature to compare.

Fix #443

This solution is a temporary solution as it solves current
problem, but there is still the case for `None` values.

The best solution for this case would be wrapping all the values
on assignment and give it a full path signature to compare.
if (
existing_value is not None
and key == full_path[-1]
and existing_value is value
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is still buggy as this comparison cannot handle None types.

To improve this we need to create a way to have a unique signature for each value.

@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

Merging #444 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #444   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          846       846           
=========================================
  Hits           846       846           
Impacted Files Coverage Δ
dynaconf/utils/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8998cac...2db3537. Read the comment docs.

@rochacbruno rochacbruno merged commit 8a987b9 into master Oct 8, 2020
@rochacbruno rochacbruno deleted the 443 branch October 8, 2020 02:31
rochacbruno added a commit that referenced this pull request Oct 8, 2020
Shortlog of commits since last release:

    Bruno Rocha (13):
          Release version 3.1.1
          Update diagram images
          Update docs/release_notes
          Fixing prospector warnings. (#425)
          Fix mkdocs config problem found in #423
          Signed in for https://xscode.com/rochacbruno/dynaconf (#426)
          Remove links to outdated issues from guidelines
          Fix colors and KEyError handling on cli.py (#429)
          Fix #434 setenv failing to unset LazyValues (#437)
          Fix #432 no need for warning when env is missing on a file (#438)
          Add test to ensure fix #430 (#439)
          Close #284 not a bug (#440)
          Fix #443 object merge with same value on same level keys (#444)

    dependabot-preview[bot] (6):
          Bump mkdocs-material from 5.3.2 to 5.5.13 (#423)
          Bump pymdown-extensions from 7.1 to 8.0 (#422)
          Bump mkdocs-material-extensions from 1.0 to 1.0.1 (#427)
          Bump pymdown-extensions from 8.0 to 8.0.1 (#435)
          Bump mkdocs-material from 5.5.13 to 6.0.1 (#436)
          Bump mkdocs-material from 6.0.1 to 6.0.2 (#442)
@daviddavis
Copy link

dance_with_cat

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.

[bug] Setting values being stripped out after merge
2 participants