Some suggestions in Example section #334
Conversation
|
@fabiojose added "Closes: #332" to the first comment so github will close it automagically - hope that's ok. |
|
@duglin, @cneijenhuis, this PR is that ok to you? |
|
LGTM minor request, s/Example/Examples/ |
| @@ -198,6 +198,9 @@ help intermediate gateways determine how to route the events. | |||
| semantics behind the data encoded in the URI is event producer defined. | |||
| * Constraints: | |||
| * REQUIRED | |||
| * Examples | |||
| * urn:event:from:cloudevents/spec/pull/123 | |||
| * urn:event:from:myapi/resourse/123 | |||
There was a problem hiding this comment.
Instead of two urn examples, what do you think about having each for a different scheme?
https://github.com/cloudevents/spec/pull/123urn:event:from:myapi/resourse/123mailto:cncf-wg-serverless@lists.cncf.io
There was a problem hiding this comment.
Good point @cneijenhuis, I will do like this.
spec.md
Outdated
| @@ -290,3 +293,24 @@ The following example shows a CloudEvent serialized as JSON: | |||
| "data" : "<much wow=\"xml\"/>" | |||
| } | |||
| ``` | |||
| The following example shows a CloudEvent with JSON as `data` and relative URI in `source`: | |||
There was a problem hiding this comment.
In the spirit of keeping PRs as small as possible, I'd suggest to remove the second example (and rather open up a follow-on PR).
Otherwise, this PR mixes two topics:
- Should we enforce the URI scheme, or should we loosen it up to URI-references?
- What is a good second example?
These aren't really related. Personally, I think the second example should differ more from the first one - but it is a bit off-topic.
There was a problem hiding this comment.
Right, understood!
I will keep this PR small and remove the second example.
|
@fabiojose can you sign your commits? |
|
@duglin my bad! But now the ci is not passing, because this: ./extensions/distributed-tracing.md: Can't load url: https://w3c.github.io/distributed-tracing/report-trace-context.html
./extensions/distributed-tracing.md: Can't load url: https://w3c.github.io/distributed-tracing/report-trace-context.html#field-value
./extensions/distributed-tracing.md: Can't load url: https://w3c.github.io/distributed-tracing/report-trace-context.html#header-value |
|
yep - I'm trying to find the new URLs for those - the w3c site did some reorg the other day |
|
LGTM |
|
@fabiojose can you rebase? |
|
Approved on 11/1 call - so after rebase we can merge |
Signed-off-by: Fabio José <fabiojose@gmail.com>
- Add examples in source section and update the JSON examples. Signed-off-by: Fabio José <fabiojose@gmail.com>
Remove the second example of serialized JSON and update the `source` examples list. Signed-off-by: Fabio José <fabiojose@gmail.com>
|
never mind - I was able to do it. :-) |
Signed-off-by: Fabio José fabiojose@gmail.com
PR related to issue #332
Closes: #332