-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
feat(content-blog): infer blog post date from git history #6593
Conversation
✔️ [V2] 🔨 Explore the source changes: eff155a 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/62009019eca05f0008347cc1 😎 Browse the preview: https://deploy-preview-6593--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6593--docusaurus-2.netlify.app/ |
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.
Thanks, looks like a useful improvement, much better than birthtime 👍
packages/docusaurus-plugin-content-blog/src/__tests__/index.test.ts
Outdated
Show resolved
Hide resolved
@slorber, @Josh-Cena next step is to write tests for the new gitUtils and refactor the existing tests to match, so don't look at the CI for now. But is this what you had in mind? |
LGTM |
Alright. I'll finish it then. |
packages/docusaurus-plugin-content-blog/src/__tests__/index.test.ts
Outdated
Show resolved
Hide resolved
Because it's only used in tests.
Thanks, that looks good enough to give a try 👍 |
just found this - nice work, would be great to expose "date created" and "date updated" concepts as a way to encourage people to keep blogposts updated |
I totally agree! |
Apparently, this change produces a bug for sites having a custom doc gen step Discussed here: cardano-foundation/developer-portal#557 Let me know if you want to look at it @felipecrs |
@slorber just to clarify, it's just an error message (which does not cause the pipeline to fail). If it's failing, it must be due to something else. |
Motivation
I started my Docusaurus website without reading the documentation about the blog. I didn't know I had to manually add the blog post date to my blog posts, because I was expecting that Docusaurus would infer it automatically, as it does similar stuff in the documentation for the last updated date.
After I created my third blog post, I realized that they were all unordered and all the dates were not correct (because it was taking the creation time of the file, which in CI will always be the same for all the files).
So, while I could simply set the date in the files by myself, I think that making docusaurus automatically infer the correct date would be a good idea.
By the way, there is some tinkering on why I'm using the committer date rather than author date for the date. It is because the chances are that the blog post is created when the commit is committed. You could for example, create a commit today, and keep amending it until next week to finish, and therefore author date would be behind).
This works perfectly if the blog post is merged with the GitHub Squash strategy, or if you use the rebase strategy and keep amending the original commit until you merge. The latter is what happens on Gerrit by the way.
It would be even better if there was a way to get the merge date, so that the result would be sharper even in situations different than the mentioned above, like when using Merge or Rebase (with multiple commits) strategies. But this information is outside Git, I guess. Anyway, I think that the committer date is a good enough approximation, especially when compared to the previous approach which is totally broken.
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
Tests were changed and are passing. The fixed date is the original committer date of the fixture file, which is extremely unlikely to change.
EDIT: the date differs depending on the depth used to clone the repository.
Related PRs
This includes similar optimizations made in #6592.