json values are people too, two #968
Merged
Conversation
This file contains 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
denismakogon
approved these changes
May 2, 2018
treeder
suggested changes
May 2, 2018
There was a problem hiding this comment.
I'm pretty sure this is incorrect.
If the contentType value is "application/json", or any media type with a structured +json suffix, the implementation MUST translate the data attribute value into a JSON value, and set the data member of the envelope JSON object to this JSON value.
Will also break ruby fdk too.
the spec and the json spec seem to have different opinions about... a lot. sigh. |
|
I filed cloudevents/spec#184 in hopes the cloud events gods will shine their benevolent light upon us |
adds a test for the protocol dumping of a request to the containert stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff overall seems a little weird.
* fixes content type issue around json checking (since a string is also a json
value, we can just decode it, even though it's wasteful it's more easily
correct)
* doesn't force all json values to be map[string]interface{} and lets them be
whoever they want to be. maybe their dads are still proud.
closes #966
|
@treeder i think we agree meow ? |
treeder
approved these changes
May 8, 2018
|
thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
related: fnproject/fdk-go#26
adds a test for the protocol dumping of a request to the container stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff still seems a little funky.
value, we can just decode it, even though it's wasteful it's more easily
correct)
whoever they want to be. maybe their dads are still proud.
closes #966