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

Last Updated Time Misleading #1015

Closed
JoelMarcey opened this issue Oct 3, 2018 · 4 comments
Closed

Last Updated Time Misleading #1015

JoelMarcey opened this issue Oct 3, 2018 · 4 comments
Labels
status: claimed Issue has been claimed by a contributor who plans to work on it.

Comments

@JoelMarcey
Copy link
Contributor

🐛 Bug Report

The last updated time is misleading and possibly wrong. We don't want to take into account versioning in last updated -- only content changes. Also, the folder move to v1 and v2 might have made things out of whack a bit.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. Go to https://docusaurus.io/docs/en/installation - look at last updated time
  2. Go to https://docusaurus.io/docs/en/1.0.12/installation - look at last updated time
  3. They are the same. They shouldn't be in my opinion.

Expected behavior

The correct last updated time based on content changes

Actual Behavior

Every doc has the same last updated time.

Reproducible Demo

https://docusaurus.io

@JoelMarcey
Copy link
Contributor Author

@yangshun - do you agree that the times are misleading?

I am going to assign this to @fiennyangeln as part of her work on #980. She can fix this and use that same fix to put the last contributor of each document.

@JoelMarcey JoelMarcey added mentorship status: claimed Issue has been claimed by a contributor who plans to work on it. labels Oct 3, 2018
@yangshun
Copy link
Contributor

yangshun commented Oct 3, 2018

Yes, I agree they are misleading, but they are not technically wrong, because moving a file is indeed updating it strictly speaking.

I'm not sure how else we can do better. Perhaps we can find the latest commit where it's not a movement but a modification.

@JoelMarcey
Copy link
Contributor Author

Perhaps we can find the latest commit where it's not a movement but a modification.

Exactly. I chatted with @fiennyangeln this morning and that is exactly what she was going to look at doing. I think that would be more useful information to someone reading the docs.

@fiennyangeln
Copy link
Contributor

@yangshun I feel like currently even if it's not moved, it will show the time when we start a new version, not when the content last changed before we create the version

fiennyangeln added a commit to fiennyangeln/Docusaurus that referenced this issue Oct 8, 2018
fiennyangeln added a commit to fiennyangeln/Docusaurus that referenced this issue Oct 8, 2018
yangshun pushed a commit that referenced this issue Oct 14, 2018
* Fix last updated time misleading, only show when file content change
or otherwise when it is first created

Fix #1015

* Fix prettier

* - Simplify regex
- Uses shelljs instead of cross-spawn
- Make logic clearer

* Add test when repositories is moved

* Use shell.exec mock

I initially try to mock the whole shelljs. But it returns error
shell.exec is not a function when i try to provide the mockResolvedValue
I think it is because of the inner code of shelljs who run a forEach to
require each of its method which make it a promise. I tried moving the
jest.mock inside beforeAll and also adding babel-dynamic-import but it did
not solve the problem. In the end, I decided to just mock shelljs.exec since
it is the only function used anyway
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: claimed Issue has been claimed by a contributor who plans to work on it.
Projects
None yet
Development

No branches or pull requests

3 participants