Specify the media type for proto representations#346
Merged
duglin merged 2 commits intocloudevents:masterfrom Nov 29, 2018
Merged
Specify the media type for proto representations#346duglin merged 2 commits intocloudevents:masterfrom
duglin merged 2 commits intocloudevents:masterfrom
Conversation
Signed-off-by: Rachel Myers <rachelmyers@google.com>
Merged
duglin
reviewed
Nov 8, 2018
| | Integer | int_value | ||
| | Any | Not applicable. Any is the enclosing CloudEventAny message itself | ||
|
|
||
| Protocol Buffer representations of CloudEvents use the media type `application/cloudevents+proto`. |
Collaborator
There was a problem hiding this comment.
I think this needs a MUST in there somewhere
Contributor
Author
There was a problem hiding this comment.
Maybe. I left it out so the wording here matches the wording in the JSON spec:
Such a representation uses the media type
application/cloudevents+json.
Collaborator
|
Ah good catch. We should fix it in both places
Thanks,
-Doug
Sent from my iPhone
… On Nov 15, 2018, at 7:43 AM, Rachel Myers ***@***.***> wrote:
@rachelmyers commented on this pull request.
In protobuf-format.md:
> @@ -88,6 +88,7 @@ The CloudEvents type system MUST be mapped into the fields of
| Integer | int_value
| Any | Not applicable. Any is the enclosing CloudEventAny message itself
+Protocol Buffer representations of CloudEvents use the media type `application/cloudevents+proto`.
Maybe. I left it out so the wording here matches the wording in the JSON spec: Such a representation uses the media type application/cloudevents+json.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Contributor
|
Unfortunately the example code needs tweaking to account for the recent changes to property naming ; eg eventTime -> time, cloudEventsVersion -> specversion etc. |
Contributor
Author
|
@JemDay Totally true. It's outside the scope of this PR adding media type definition, but I'll make a follow up. |
Contributor
|
@rachelmyers - I'm happy to open PR for that if you wish ... let me know ... |
Contributor
Author
|
@JemDay That would be great; thanks. |
Signed-off-by: Rachel Myers <rachelmyers@google.com>
rachelmyers
force-pushed
the
proto-media-type
branch
from
2f954c5 to
31d76c1
Compare
duglin
reviewed
Nov 16, 2018
| Each CloudEvents event can be wholly represented as a JSON object. | ||
|
|
||
| Such a representation uses the media type `application/cloudevents+json`. | ||
| Such a representation MUST use the media type `application/cloudevents+json`. |
Collaborator
There was a problem hiding this comment.
Thanks for adding this to the PR!
Collaborator
|
LGTM |
Contributor
|
+1 |
Collaborator
|
Approved on 11/29 call |
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
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.
This PR addresses a concern raised by @JemDay that the proto spec didn't specify which media type should be used in the envelopes for messages using the proto spec.
This follows the convention of the json spec and specifies the media type of
application/cloudevents+proto.