feat: add last contributor to each document by fiennyangeln · Pull Request #980 · facebook/docusaurus

fiennyangeln · 2018-09-25T11:24:51Z

Motivation

Create Contributor List for each documents (#858)

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Run yarn build on the website -- it will update the author list in the metadata

Related PRs

None

docusaurus-bot · 2018-09-25T11:26:50Z

Deploy preview for docusaurus-preview ready!

Built with commit 821331e

https://deploy-preview-980--docusaurus-preview.netlify.com

endiliey

The PR preview for v2 is available on
https://deploy-preview-980--munseo-preview.netlify.com/

To be honest I don't agree with this feature on v2 as this would best be left as a plugin and not every user would want it. I personally don't want to see bloated contributor A xxxx% on the docs

JoelMarcey · 2018-09-25T17:03:48Z

@endiliey - does the Netlify preview actually show the contributor list? I don't see it.

This was a v1 feature request that would have been toggled by a siteConfig option. Can we just add this as a v1 update then instead?

endiliey · 2018-09-25T17:05:24Z

@JoelMarcey

It's at the bottom, hahaha.

I do agree if it's on v1. But I think for v2 our paradigm is that we don't want a lot of siteConfig and try to keep things as simple as possible especially on UI related things like this.

In addition, I think it's not a good idea to add the field into the docs metadata itself.

In the UI itself, we are exposed metadata.source which is an absolute path to this current docs file, we can just git blame that. So it's more efficient :)

This reverts commit a76a887.

This reverts commit c0cc79f.

fiennyangeln · 2018-09-29T13:30:06Z

@JoelMarcey @endiliey I finished doing git blame on the filePath, but i just realized it may not show the correct contributors name, instead, it show the name of whoever cut the version and the changes after it. It is because when we copy a file, we dont use git command so the file history kinda disappear.

My proposed solution would be:

Get the contributor name when we cut a version (put it in metadata or some other places)
Create a one time script to generate the author of each docs to be put in the metadata: for every file in the versioned_docs -> get the time of the first commit which would be the time when the version is made, go to the /docs/[filename], get the snapshot of the file at that moment, and get the contributors at that time -> save to metadata when we made the file.

But, we still need to think of edge case like how if the readme inside versioned_docs get changed, how shall we combine the result from /docs and /versioned_docs?

endiliey · 2018-09-30T09:14:33Z

That is correct. Do note we have version fallback on v1 & that make things even more complicated.

To be honest I don't agree with this feature on v2 as this would best be left as a plugin and not every user would want it. I personally don't want to see bloated contributor A xxxx% on the docs

Maybe I should say its the same for v1. But ofc it could be a good addition to have.

WDYT @JoelMarcey @yangshun

JoelMarcey · 2018-09-30T15:04:31Z

I think this feature was always intended for v1.

I have to scope change thoughts two this, if we believe contributors are becoming too difficult or cumbersome:

One thought here is that we could change the scope of this to be where the last content commit to the docs is shown with a link to the commit.
We have a link to the full history of changes for the doc. Basically have a link that brings you directly to something like this: https://github.com/facebook/Docusaurus/commits/master/docs/api-site-config.md

fiennyangeln · 2018-10-15T01:45:25Z

Now that the last updated time is merged, I will start working back on this. Do we want the last updated by field instead? @yangshun @JoelMarcey

- Rename css class to be more general

- s/getGitlastupdated/getGitLastUpdatedTime - refactor part in getGitLastUpdated[Time|By] that overlaps - remove getAuthorInformation

JoelMarcey

I ❤️ the way this looks on the preview.

Is there anyway to have a link to the commit represented by the last update time and last committer?

yangshun

Code looks good, just have some suggestions regarding formatting!

yangshun · 2018-10-15T18:00:17Z

v1/lib/core/DocsLayout.js

-                <div className="docLastUpdateTimestamp">
+            {(updateTime || updateAuthor) && (
+              <div className="docLastUpdate">
+                {updateTime && (


Let's use a combined format:

Only enableUpdateTime: "Last updated on 2018-10-15 01:23"

Only enableUpdateBy: "Last updated by @fiennyangeln"

Both enableUpdateTime and enableUpdateBy: "Last updated on 2018-10-15 01:23 by @fiennyangeln"

WDYT?

It's nice 😍 ! I will implement this.

yangshun

Gave a more thorough review. Please let me know if you have questions!

yangshun · 2018-10-15T18:39:31Z

v1/lib/core/utils.js


  // Wrap in try/catch in case the shell commands fail (e.g. project doesn't use Git, etc).
  try {
+    const format = GIT_LAST_UPDATED_TYPE.TIME === type ? '%ct' : 'author=%an';


Do we need the author here?

Let's do away with this format variable. Instead, we could do

git log --follow --summary --format="%ct, %an" <file>

which would generate something like:

1539553317, Yangshun Tay 1539457591, Yangshun Tay 1539185600, Yangshun Tay 1538339532, Fienny Angelina 1537426859, Marvin Heilemann 1537330146, Yangshun Tay 1537203487, Yangshun Tay copy package.json => v1/package.json (81%) 1537169695, Yangshun Tay 1537157761, Yangshun Tay 1537157671, Endilie Y 1537157183, endiliey

and then parse each line for the timestamp and author using a regex that extracts the timestamp and the author. Exampe regex here - https://regex101.com/r/O0HQrc/1

This method will just return both the relevant timestamp and author as an object { timestamp: 12345, author: 'Fienny' }, and it's up to the callsite to use whichever field they need.

I initially put the author to differentiate author vs commit summary since both are just plain string 😁 Wdyt about making it stronger like ^(\d{10}), (.+)$ ? It can last until 2038 based on Google

yangshun · 2018-10-15T18:42:35Z

v1/lib/core/utils.js

+
+  const records = getGitLastUpdated(filepath, GIT_LAST_UPDATED_TYPE.TIME);
+
+  const timeSpan = records.find((item, index, arr) => {


This finding logic can be shifted into the getGitLastUpdated method.

And getGitLastUpdatedTime will in the end just become:

function getGitLastUpdatedTime(filepath) { const commit = getGitLastUpdated(filepath); if (!commit) { return null; } const date = new Date(parseInt(commit.timestamp, 10) * 1000); return date.toLocaleString(); }

yangshun · 2018-10-15T18:59:16Z

Is there anyway to have a link to the commit represented by the last update time and last committer?

It's possible but not sure how much value it provides. Going directly to the list of commits for the file would be more valuable. We could do this in a follow up PR though.

JoelMarcey · 2018-10-15T19:05:31Z

It's possible but not sure how much value it provides. Going directly to the list of commits for the file would be more valuable.

I think we can debate which is more valuable, but I am ok with going to a list of commits (in fact, I may have suggested even that option before). I am quite certain though, having a link to somewhere makes more sense than having no link at all.

I would totally expect as a user to have a link to some commit or a list of commits for the file or something.

-Refactor the utils, combine lastupdatedtime and lastupdatedby -Replace the test

For more clarity and to make relationship more clear

…n/Docusaurus into fiennyangeln-add-contributor-list

yangshun · 2018-10-18T07:36:56Z

docs/api-site-config.md

 #### `fonts` [object]
 Font-family CSS configuration for the site. If a font family is specified in `siteConfig.js` as `$myFont`, then adding a `myFont` key to an array in `fonts` will allow you to configure the font. Items appearing earlier in the array will take priority of later elements, so ordering of the fonts matter.

+#### `enableUpdateBy` [boolean]


This is a wrong place to put this. Now it's in between the fonts config.

yangshun · 2018-10-18T07:37:34Z

docs/api-site-config.md

 Font-family CSS configuration for the site. If a font family is specified in `siteConfig.js` as `$myFont`, then adding a `myFont` key to an array in `fonts` will allow you to configure the font. Items appearing earlier in the array will take priority of later elements, so ordering of the fonts matter.

+#### `enableUpdateBy` [boolean]
+An option to enable the docs showing last update time. Set to `true` to show a line at the bottom right corner of each doc page as `Last updated by <Author Name>`.


Copy paste error... Should be about last author not last update time.

yangshun

There are some small nits which I have made on your behalf. Additionally, I changed the updated time format to show just the date as the timing is not that crucial. Thanks for making this happen!

add contributor list to each document

c0cc79f

facebook-github-bot added the CLA Signed Signed Facebook CLA label Sep 25, 2018

handle case where there is no github repo

a76a887

endiliey reviewed Sep 25, 2018

View reviewed changes

endiliey requested a review from yangshun September 25, 2018 17:03

fiennyangeln added 3 commits September 29, 2018 10:35

Move to v1

370f398

Revert "handle case where there is no github repo"

2801979

This reverts commit a76a887.

Revert "add contributor list to each document"

f4199c0

This reverts commit c0cc79f.

JoelMarcey mentioned this pull request Oct 3, 2018

Last Updated Time Misleading #1015

Closed

fiennyangeln added 4 commits October 15, 2018 22:09

Merge branch 'master' into add-contributor-list

2c54925

Add last updated by field

88f5d89

-Include enableUpdateBy in the config

5b2f9ab

- Rename css class to be more general

Fix lint

9fedb24

fiennyangeln force-pushed the add-contributor-list branch from bf676bf to b8487a7 Compare October 15, 2018 16:24

Refactor :

4bb321f

- s/getGitlastupdated/getGitLastUpdatedTime - refactor part in getGitLastUpdated[Time|By] that overlaps - remove getAuthorInformation

fiennyangeln force-pushed the add-contributor-list branch from b8487a7 to 4bb321f Compare October 15, 2018 16:25

JoelMarcey reviewed Oct 15, 2018

View reviewed changes

yangshun suggested changes Oct 15, 2018

View reviewed changes

fiennyangeln added 2 commits October 17, 2018 22:42

-Modify the display logic

0b753b2

-Refactor the utils, combine lastupdatedtime and lastupdatedby -Replace the test

Modify docs based on the display change

dc6f24f

fiennyangeln force-pushed the add-contributor-list branch from f9d02dc to dc6f24f Compare October 17, 2018 14:53

JoelMarcey and others added 2 commits October 17, 2018 10:20

Update docs for url and baseurl

a39d2b0

For more clarity and to make relationship more clear

Update support for Web App Manifest (facebook#1046)

4a7e1ea

yangshun changed the title ~~add contributor list to each document~~ feat: add last contributor to each document Oct 18, 2018

Merge branch 'add-contributor-list' of https://github.com/fiennyangel…

009fa62

…n/Docusaurus into fiennyangeln-add-contributor-list

yangshun reviewed Oct 18, 2018

View reviewed changes

misc: update nits

3f84e4f

yangshun approved these changes Oct 18, 2018

View reviewed changes

misc: update nits

821331e

yangshun merged commit 8cf9afe into facebook:master Oct 18, 2018

NikolayS mentioned this pull request Jul 7, 2023

[Snyk] Security upgrade babel-loader from 8.3.0 to 9.1.3 NikolayS/docusaurus#91

Open


		const records = getGitLastUpdated(filepath, GIT_LAST_UPDATED_TYPE.TIME);

		const timeSpan = records.find((item, index, arr) => {

Uh oh!

Conversation

fiennyangeln commented Sep 25, 2018

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

Uh oh!

docusaurus-bot commented Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

endiliey left a comment

Choose a reason for hiding this comment

Uh oh!

JoelMarcey commented Sep 25, 2018

Uh oh!

endiliey commented Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fiennyangeln commented Sep 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

endiliey commented Sep 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoelMarcey commented Sep 30, 2018

Uh oh!

fiennyangeln commented Oct 15, 2018

Uh oh!

JoelMarcey left a comment

Choose a reason for hiding this comment

Uh oh!

yangshun left a comment

Choose a reason for hiding this comment

Uh oh!

yangshun Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

fiennyangeln Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

yangshun left a comment

Choose a reason for hiding this comment

Uh oh!

yangshun Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

fiennyangeln Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangshun Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

yangshun commented Oct 15, 2018

Uh oh!

JoelMarcey commented Oct 15, 2018

Uh oh!

yangshun Oct 18, 2018

Choose a reason for hiding this comment

Uh oh!

yangshun Oct 18, 2018

Choose a reason for hiding this comment

Uh oh!

yangshun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

docusaurus-bot commented Sep 25, 2018 •

edited

Loading

endiliey commented Sep 25, 2018 •

edited

Loading

fiennyangeln commented Sep 29, 2018 •

edited

Loading

endiliey commented Sep 30, 2018 •

edited

Loading

fiennyangeln Oct 16, 2018 •

edited

Loading