Skip to content

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

JSONByteSlice currently doesn't implement UnmarshalJSON. This is pretty unusual (and is something that is desired for #3217.

How this works

  1. Correctly supports nil
  2. Implements UnmarshalJSON

How this was tested

  • Added new unit tests
  • Fixed tests that relied on marshaling nil as "0x"

@StephenButtolph StephenButtolph added this to the v1.11.11 milestone Sep 3, 2024
@StephenButtolph StephenButtolph self-assigned this Sep 3, 2024
Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

No need for me to re-review the changes so approving.

require.NoError(err)
require.Equal(test.expectedJSON, string(jsonBytes))

var unmarshaled JSONByteSlice
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to support overwriting existing data (related to my comment about unmarshalling "null")? If so, set this to something non-empty to begin with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an explicit test for the null behavior

@StephenButtolph StephenButtolph added this pull request to the merge queue Sep 3, 2024
Merged via the queue into master with commit 833b3a0 Sep 3, 2024
21 checks passed
@StephenButtolph StephenButtolph deleted the standardize-hex-json branch September 3, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants