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
Hotfix/optional leaf object #1865
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, is there a chance for you to add tests for the fixes? Also, I see code related to ES, does it belong to the same problem?
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1865 +/- ##
==========================================
- Coverage 84.68% 84.67% -0.02%
==========================================
Files 136 136
Lines 9261 9268 +7
==========================================
+ Hits 7843 7848 +5
- Misses 1418 1420 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
+1 for adding a test, once there is a test I could take over for the review if you want @JoanFM |
Perfect! |
Also @oytuntez, please be aware that we will need to have all the commits |
This PR aims to improve
DocList
fields that areOptional
or that are givenNone
default value. Giving empty[]
as default value would create entries in the subindex (with just the ID field, no other data), so I opted forNone
.The biggest issue that this PR aims to solve is
DocList
type fields would be required no matter if they have a default value or are marked asOptional
.