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.

Sign up for GitHub

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

Jump to bottom

Refactored RDF examples for simplicity #688

Merged
merged 22 commits into from Jun 14, 2019
Merged

Refactored RDF examples for simplicity #688

merged 22 commits into from Jun 14, 2019

Conversation

vcharpenay
Copy link
Contributor

  • introductory uses an example namespace, to remove unnecessary details
  • section 7 was edited, there are now 2 examples for data schema annotations (29, 30).
  • appendix was modified accordingly (text is based on examples of section 7)

@vcharpenay
Copy link
Contributor Author

Example 29 was removed, as it introduces some feature for the first time (annotation with an embedded JSON-LD context) and not enough explanation is given in the text.

@vcharpenay vcharpenay requested review from mmccool and takuki May 15, 2019 09:05
@vcharpenay
Copy link
Contributor Author

the changes of the PR should be taken into account before CR is published: TD examples with annotations are not fully consistent with the TD information model (in particular, @type is defined for instances of Form instead of InteractionAffordance).

@mmccool mmccool added Editorial Issues with no technical impact on implementations and removed by CR transition labels May 15, 2019
@mlagally mlagally self-requested a review May 15, 2019 12:59
mlagally
mlagally requested changes May 16, 2019
View reviewed changes
Copy link
Contributor

@mlagally mlagally left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed changes look very good.

A few observations:
It is not clear, if you want to indicate that SAREF and OM are recommended ontologies or just examples.
What happened to SSN/SOSA?
Please look into the name differences of the semantic types of the examples and SAREF.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@vcharpenay vcharpenay added the Propose closing Problem will be closed shortly if there is no veto. label May 17, 2019
@sebastiankb sebastiankb removed the Propose closing Problem will be closed shortly if there is no veto. label Jun 10, 2019
sebastiankb
sebastiankb reviewed Jun 10, 2019
View reviewed changes
Copy link
Contributor

@sebastiankb sebastiankb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see me remarks below

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
takuki
takuki approved these changes Jun 14, 2019
View reviewed changes
Copy link
Contributor

@takuki takuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you.

takuki
takuki reviewed Jun 14, 2019
View reviewed changes
Copy link
Contributor

@takuki takuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Section 7 should stay informative, correct?

@takuki takuki mentioned this pull request Jun 14, 2019
Closed
6 tasks
@takuki
Copy link
Contributor

takuki commented Jun 14, 2019

Can @vcharpenay also describe this changes in the Recent Changes section?

@vcharpenay
Copy link
Contributor Author

thanks for the review, @takuki. Section 7 is informative, indeed: what I did is that I removed the "This section is non-normative." statement from all sub-sections and put it only once at the beginning of the whole section, instead.

@vcharpenay
Copy link
Contributor Author

As @mlagally pointed out today in the TD call, the last merge commit was with an outdated master branch, which reintroduced errors. I reverted it and performed another merge with master, this time with the latest changes. That new merge also resolved conflicts.

mlagally
mlagally reviewed Jun 14, 2019
View reviewed changes
index.template.html Outdated Show resolved Hide resolved
@mlagally
Copy link
Contributor

This looks very good now, please consider the suggestion for a small language improvement.

There's a respec error being reported (probably not caused by your changes), but ideally you would also fix it.

@vcharpenay
Copy link
Contributor Author

vcharpenay commented Jun 14, 2019 •
edited

Commit 202e788 fixes the ReSpec issue. Thanks!

@vcharpenay
Copy link
Contributor Author

@takuki @sebastiankb @mlagally I'll merge this PR at the end of the day (~5pm CET) if you have no further comment.

@vcharpenay vcharpenay merged commit 1fd4f7f into master Jun 14, 2019
@takuki takuki mentioned this pull request Jun 18, 2019
Merged
@sebastiankb sebastiankb deleted the extension-refactor branch April 23, 2020 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@takuki takuki takuki approved these changes

@sebastiankb sebastiankb sebastiankb left review comments

@mlagally mlagally mlagally approved these changes

@mmccool mmccool Awaiting requested review from mmccool

Assignees
No one assigned
Labels
Editorial Issues with no technical impact on implementations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@vcharpenay @takuki @mlagally @sebastiankb @mmccool