-
Notifications
You must be signed in to change notification settings - Fork 228
fix: WriteJson should write data as a JSON value when dataContentType… #1162
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
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.
Thx, just a nit
Also, you need to |
@@ -165,7 +165,97 @@ func TestMarshal(t *testing.T) { | |||
"source": "http://example.com/source", | |||
}, | |||
}, | |||
"struct data v1.0": { | |||
"structured mode data v1.0": { |
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.
Can you add a test that sends a string that just happens to look like json? Just to make sure it stays a string on both ends.
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.
Are you referring to this test case?
sdk-go/v2/event/event_marshal_test.go
Line 338 in d35a778
"string data v1.0": { |
If so, it is already there and no regression (test still pass ✅ )
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.
I think this is good. Merging.
…=application/cloudevents+json (and batch) Signed-off-by: Alan Kan <[email protected]>
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.
Thx!
…=application/cloudevents+json (and batch)
#1161