Skip to content
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

Explicitly state application/json defaulting when serializing #881

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Oct 7, 2021

(I've used SHOULD rather than MUST to be consistent with deserialization. If we were starting from scratch, I'd suggest that MUST would be more appropriate, but it's probably too late to do that.)

Fixes #880

Proposed Changes

  • Explicitly state that when serializing data using the JSON event format, treat a missing datacontenttype as implicitly application/json

Release Note

If `datacontenttype` is missing when serializing in the JSON format, it should be treated as `application/json`

(I've used SHOULD rather than MUST to be consistent with deserialization. If we were starting from scratch, I'd suggest that MUST would be more appropriate, but it's probably too late to do that.)

Fixes cloudevents#880

Signed-off-by: Jon Skeet <jonskeet@google.com>
@jskeet jskeet force-pushed the more-json-format-defaulting branch from 69b3730 to 39c12dc Compare October 7, 2021 08:41
Copy link
Member

@dazuma dazuma left a comment

Choose a reason for hiding this comment

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

LGTM. I don't know how I missed this.

@duglin
Copy link
Collaborator

duglin commented Oct 19, 2021

Approved on the 10/14 call

@duglin duglin merged commit f02c85e into cloudevents:master Oct 19, 2021
@jskeet jskeet deleted the more-json-format-defaulting branch November 23, 2021 08:58
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.

3 participants