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

Update the examples based on changes of spec.md #341

Closed
wants to merge 1 commit into from
Closed

Update the examples based on changes of spec.md #341

wants to merge 1 commit into from

Conversation

fabiojose
Copy link
Contributor

Signed-off-by: Fabio José fabiojose@gmail.com

Based on changes made in spec.md, in source elements.

Signed-off-by: Fabio José <fabiojose@gmail.com>

Based on changes made in spec.md, in `source` elements.
@duglin
Copy link
Collaborator

duglin commented Nov 5, 2018

While it does no harm to make these changes, I will point out that it is not necessary since 'source' is a URI-reference which allows for /path type of values.

@fabiojose
Copy link
Contributor Author

@duglin I agree with you, but what do you think if the audience of this spec will get a little bit confusing? They get examples of source in spec.md and different ones in json-format.md. If they try to run the JSON files against the JSON Schema, these files will be invalid.

What do you think if we add additional examples here too?

@duglin
Copy link
Collaborator

duglin commented Nov 6, 2018

I believe that #338 should fix the schema to support uri-reference and help with the confusion because we'll be using URI-Reference throughout the specs.

I actually wouldn't mind if we had a combination of samples that used a urn:... form and a /... form just to reinforce that both are valid. A lot of times people don't read the specs very closely (shocking, I know!) and they look at examples for guidance - so if all they see are urn:... they may jump to the wrong conclusion.

@duglin
Copy link
Collaborator

duglin commented Nov 6, 2018

But to be clear, I don't think its a major issue either way.

@fabiojose
Copy link
Contributor Author

@duglin Good points!

Closed!

@fabiojose fabiojose closed this Nov 6, 2018
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.

2 participants