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

[DIN-749] Add feature to delete nulls in overrides #30

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Miller-GS
Copy link

@Miller-GS Miller-GS commented Dec 12, 2023

Why? πŸ“–

There is currently no way to remove a key that was previously added using overrides. This is a limitation that we can solve by removing null values.

What? πŸ”§

  • Add feature to remove key if the value in the override is None
  • Add unit test for new case
  • Update version to 1.0.4

Type of change πŸ—„οΈ

  • New feature (non-breaking change which adds functionality)

How everything was tested? πŸ“

Unit tests

Checklist πŸ“

  • I have added labels to distinguish the type of pull request.
  • My code follows the style guidelines of this project (docstrings, type hinting and linter compliance);
  • I have performed a self-review of my own code;
  • I have made corresponding changes to the documentation;
  • I have added tests that prove my fix is effective or that my feature works;
  • I have made sure that new and existing unit tests pass locally with my changes;

@Miller-GS Miller-GS added the enhancement New feature or request label Dec 12, 2023
@Miller-GS Miller-GS self-assigned this Dec 12, 2023
Copy link

sonarcloud bot commented Dec 12, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Comment on lines +120 to +121
elif value is None:
source.pop(key, None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we want to choose wether the key is removed or it's persisted with the None value? There may be some situations where we would like to keep the key with the None, or am I being to picky?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants