-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
Added schema.org JSON-LD to lesson #226
base: main
Are you sure you want to change the base?
Conversation
@njall I moved where the JSON-LD is included. Could you check for me? |
_includes/schema_org.html
Outdated
"timeRequired": "{{site.data.lesson.timeRequired}}", | ||
"learningResourceType": "{{site.data.lesson.learningResourceType}}", | ||
"citation": "{{site.data.lesson.citation}}", | ||
"dateCreated" : "{{site.data.lesson.dateCreated}}" |
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.
It's not super clear what dateCreated
refers to. First commit? Last commit? Last release?
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.
That would be the first commit after forking the lesson example.
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.
Why not use the date from the last commit? See 40f1dfa
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.
Last commit is not when the lesson has been created, it's when the lesson has been updated.
Why removing this field in
40f1dfa
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.
@njall Does schema.org have a field for date updated? If not, can we use dateCreated
for it?
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.
Yes, we have a field called dateModified that would be good for last commit.
Date Created could be the first commit after the repo was cloned.
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.
OK. We can't automate dateCreated
.
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.
I think that's ok, since it will happen once in a lifetime of the training meterial.
{% endfor %} | ||
], | ||
"timeRequired": "{{site.data.lesson.timeRequired}}", | ||
"learningResourceType": "{{site.data.lesson.learningResourceType}}", |
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.
How is it different from genre
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.
"Genre" is replaced by "About" now. LerningResourcesType, that can be video lecture, e-learning module, handout etc.
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.
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 an open list. More info here: https://schema.org/learningResourceType
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.
Yes, it's an open list for now. I have been tempted to create a vocab but making and maintaining an exhaustive list would be an overhead to big for its benefit right now.
_layouts/base.html
Outdated
@@ -22,7 +22,8 @@ | |||
<script src="https://oss.maxcdn.com/html5shiv/3.7.2/html5shiv.min.js"></script> | |||
<script src="https://oss.maxcdn.com/respond/1.4.2/respond.min.js"></script> | |||
<![endif]--> | |||
<title>{{ site.title }}{% if page.title %}: {{ page.title }}{% endif %}</title> | |||
<title>{{ site.data.lesson.title }}{% if page.title %}: {{ page.title }}{% endif %}</title> | |||
{% include schema_org.html %} |
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.
How does it affect the header of a lesson?
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.
It does not affect the header.
"@type": "Person", | ||
"name": "{{contributor}}" | ||
}{% if forloop.last %}{%else%},{%endif%} | ||
{% endfor %} |
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.
Author and contributor are vague terms as lessons are being developed by the community. There is an initial author and then there is an army of contributors. Giving proper credits is important, but it should probably happen on a separate page
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.
The distinction between author and contributor:
Author: Preparation, creation and/or presentation of the published work, specifically writing the initial draft;
Contributor: a critical review, commentary or revision;
We need this information in metadata. We may reuse it to display on the separate page.
I like the added structure this PR adds but I'm not sure about some of the categories. |
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.
Please see my comments :)
a29a101
to
b13ce0d
Compare
@njall and @mkuzak I rebased this pull request. Could you answer @maxim-belkin's questions? |
Use the time that the lesson is build on GitHub
This is a fork of #41.