Skip to content

serialise empty values as null (JSON) or empty element (XML) instead of the zero value of the type#3

Open
Southclaws wants to merge 4 commits into
leighmcculloch:mainfrom
Southclaws:main
Open

serialise empty values as null (JSON) or empty element (XML) instead of the zero value of the type#3
Southclaws wants to merge 4 commits into
leighmcculloch:mainfrom
Southclaws:main

Conversation

@Southclaws
Copy link
Copy Markdown

See #2 for proposal and justification behind this change.

Copy link
Copy Markdown
Owner

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this fix!

Comment thread example_test.go
}

func Example_jsonUnmarshalEmpty() {
func TestExample_jsonUnmarshalEmpty(t *testing.T) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could we keep this as an example? Examples already assert on the output.

Comment thread example_test.go
// Uint: false
// Uintptr: false

t.Run("unmarshal null values to empty", func(t *testing.T) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you put this in a new example? Not a test please. The examples show up in docs and so help people understand how the library works.

Comment thread optional.go
// wrapped by this optional.
func (o Optional[T]) String() string {
return fmt.Sprintf("%v", o.ElseZero())
if v, ok := o.Get(); ok {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The comment for the function needs updating as it says if no value is set that the zero value of the type is returned, which is no longer the case.

sruehl added a commit to sruehl/go-optional that referenced this pull request Aug 16, 2024
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.

2 participants