-
Notifications
You must be signed in to change notification settings - Fork 6
Improve examples. Remove unnecessary details. #21
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
Improve examples. Remove unnecessary details. #21
Conversation
README.md
Outdated
| Time : 4/26/2021 9:00:45 AM | ||
| Type : com.example.object.deleted.v2 | ||
| ``` | ||
| #### Add Custom Format Data to a CloudEvent object |
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.
Is this supposed to live under the XML example (heading level)?
Can we also provide an example output as we do for JSON/XML above?
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.
No, it's supposed to be separate.
Adding the output
| ### Send CloudEvent object to HTTP server | ||
| ```powershell | ||
| Invoke-WebRequest -Method POST -Uri 'http://my.cloudevents.server/' -Headers $cloudEventBinaryHttpMessage.Headers -Body $cloudEventBinaryHttpMessage.Body | ||
| Invoke-WebRequest -Method POST -Uri 'http://my.cloudevents.server/' -Headers $cloudEventStructuredHttpMessage.Headers -Body $cloudEventStructuredHttpMessage.Body |
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.
Since the example uses a Structured CloudEvent, are Headers actually used at all in this example? What is contained in the Headers of a structured CE? Just curious :)
Do we need to show two examples, e.g. using Headers+Body for binary mode?
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.
Headers follow the https://github.com/cloudevents/spec/blob/v1.0.1/http-protocol-binding.md#3-http-message-mapping. In structure mode the ContentType header is application/cloudevents
Do we need to show two examples, e.g. using Headers+Body for binary mode?
Not sure. I can leave only the structure mode?
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!
embano1
left a comment
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.
GREAT work!
|
Do you want to squash the commits before merging? There's no option to squash and create a merge commit in Github directly :/ |
e22a029 to
7ace4ab
Compare
Signed-off-by: Dimitar Milov <dmilov@vmware.com>
7ace4ab to
3a9010a
Compare
embano1
left a comment
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.
LGTM 👏
Simplify READEME.md. Address issue #18
AddwithSetverbs as a result of renaming module's finctions.Signed-off-by: Dimitar Milov dmilov@vmware.com