-
Notifications
You must be signed in to change notification settings - Fork 52
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 dynamic representation of zero values in maps and slices #1154
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1154 +/- ##
==========================================
+ Coverage 51.29% 51.49% +0.19%
==========================================
Files 292 292
Lines 16322 16333 +11
==========================================
+ Hits 8373 8411 +38
+ Misses 7340 7315 -25
+ Partials 609 607 -2 ☔ View full report in Codecov by Sentry. |
func TestFromTypedStringStructWithZeroValue(t *testing.T) { | ||
type Tmp struct { | ||
Foo string `json:"foo"` | ||
Bar string `json:"bar"` |
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.
This raises an interesting point; what about omitempty
?
Can deal with this later, as it is not handled now either.
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.
Yeah fair point. I'm taking a look right now whether we need to handle it as a followup
Bundles: * Allow specifying executable in artifact section and skip bash from WSL ([#1169](#1169)). * Added warning when trying to deploy bundle with `--fail-if-running` and running resources ([#1163](#1163)). * Group bundle run flags by job and pipeline types ([#1174](#1174)). * Make sure grouped flags are added to the command flag set ([#1180](#1180)). * Add short_name helper function to bundle init templates ([#1167](#1167)). Internal: * Fix dynamic representation of zero values in maps and slices ([#1154](#1154)). * Refactor library to artifact matching to not use pointers ([#1172](#1172)). * Harden `dyn.Value` equality check ([#1173](#1173)). * Ensure every variable reference is passed to lookup function ([#1176](#1176)). * Empty struct should yield empty map in `convert.FromTyped` ([#1177](#1177)). * Zero destination struct in `convert.ToTyped` ([#1178](#1178)). * Fix integration test with invalid configuration ([#1182](#1182)). * Use `acc.WorkspaceTest` helper from bundle integration tests ([#1181](#1181)).
Bundles: * Allow specifying executable in artifact section and skip bash from WSL ([#1169](#1169)). * Added warning when trying to deploy bundle with `--fail-if-running` and running resources ([#1163](#1163)). * Group bundle run flags by job and pipeline types ([#1174](#1174)). * Make sure grouped flags are added to the command flag set ([#1180](#1180)). * Add short_name helper function to bundle init templates ([#1167](#1167)). Internal: * Fix dynamic representation of zero values in maps and slices ([#1154](#1154)). * Refactor library to artifact matching to not use pointers ([#1172](#1172)). * Harden `dyn.Value` equality check ([#1173](#1173)). * Ensure every variable reference is passed to lookup function ([#1176](#1176)). * Empty struct should yield empty map in `convert.FromTyped` ([#1177](#1177)). * Zero destination struct in `convert.ToTyped` ([#1178](#1178)). * Fix integration test with invalid configuration ([#1182](#1182)). * Use `acc.WorkspaceTest` helper from bundle integration tests ([#1181](#1181)).
Changes
In the dynamic configuration, the nil value (dyn.NilValue) denotes a value that should not be serialized, ie a value being nil is the same as it not existing in the first place.
This is not true for zero values in maps and slices. This PR fixes the conversion from typed values to dyn.Value, to treat zero values in maps and slices as zero and not nil.
Tests
Unit tests