Marshal empty as null #9
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
December 6, 2018 14:10
This change adds support for marshaling and unmarshaling “null” and reflecting that the value is not present, rather than relying on the “zero” value
Owner
|
@mwielbut thanks for the PR! I agree this makes sense. I'll create a new minor release and note that it's a breaking change. Would you mind removing the go mod files from the PR? I haven't yet switched the project over from dep but am planning to soon. |
Contributor
|
Awesome! Just removed the |
Owner
|
Cheers! Thanks again! |
mwielbut
pushed a commit
to openly-engineering/optional
that referenced
this pull request
Dec 7, 2018
* Support “null”
This change adds support for marshaling and unmarshaling “null” and reflecting that the value is not present, rather than relying on the “zero” value
* Fix to specify {{ .VariableName }}
* Add support for go modules
* Fix go mod path
* Update golden test
* Remove go.mod per pull request
markphelps
added a commit
that referenced
this pull request
May 4, 2021
* Marshal empty as null (#9) * Support “null” This change adds support for marshaling and unmarshaling “null” and reflecting that the value is not present, rather than relying on the “zero” value * Fix to specify {{ .VariableName }} * Add support for go modules * Fix go mod path * Update golden test * Remove go.mod per pull request * Add example test * Update README.md * MustGet: Add ability to get value and panic if not present Co-authored-by: Openly, Inc <[email protected]> Co-authored-by: Mateusz Wielbut <[email protected]> Co-authored-by: Matt Wielbut <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation marshals non-present values as their zero value. When the zero values are read back in, we lose the fidelity of knowing that the value was actually not-present vs. explicitly set to the zero value (the whole purpose of using the optional library). I don't think this should be expected outcome.
This pull request proposes a modification to the
UnmarshalJSONandMarshalJSONfunctions, marshaling JSONnullwhen the value isnil, and unmarshaling JSONnulltonil. This seems to be more in-line with what should be expected when writing this out to JSON.Note: this is a breaking change for dependents that rely on the existing functionality of marshaling to the zero value.
Thanks!