-
Notifications
You must be signed in to change notification settings - Fork 305
feat: add modified_time meta tag #505
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
base: master
Are you sure you want to change the base?
Conversation
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.
This plugin, as well as jekyll-feed
, make use of page.last_modified_at
. It would make more sense to use this instead of page.updated
. This way the SEO tag would match what jekyll-feed
outputs:
https://github.com/jekyll/jekyll-feed/blob/43cc5d51e30b59ab8d8ba970e42c8c437e2b233f/lib/jekyll-feed/feed.xml#L78
@ashmaroli otherwise this looks good I think? Pinging you directly as per a comment in #512 (comment) - despite not being the author of this PR, I wish to signal there is interest in this plugin.
I agree with @CookiePLMonster here. It would be better to use the |
Furthermore, even in this plugin, jekyll-seo-tag/spec/jekyll_seo_tag/drop_spec.rb Lines 298 to 315 in f449b1a
Example from my website:
|
@ashmaroli Looking at jekyll-seo-tag/lib/jekyll-seo-tag/drop.rb Lines 114 to 123 in f449b1a
article:published_time isn't exactly great either and instead it should be seo_tag.published_time - it's used for LD-JSON already, but not for the HTML template. I can prepare an alternative PR tomorrow correcting this + modification date.
|
Additionally, this duplicates #447, albeit using |
add modified date meta tag, useful for SEO