Replace ESDocs generated api-docs with api-documenter markdown #5163

robwalch · 2023-01-16T03:58:26Z

This PR will...

Replace esdocs generated api-docs with api-documenter to markdown-styles static html.

Why is this Pull Request needed?

Cleanup documentation to reflect project interfaces and types. The ESDoc generated docs do not reflect the working structure of the project or its TypeScript definitions.

This change will allow for better integration between the tailored docs/API.md guide and generated API docs created from TypeScript and doc comments already being processed by api-extractor. For more see https://api-extractor.com/pages/setup/generating_docs/

tjenkinson · 2023-01-16T17:32:09Z

Looks good but I'm not sure about checking the generated markdown into the repo. There's currently nothing to validate that the docs are actually up to date so it's easy for them to go out of sync.

How about we keep them in the gitignore and build them on ci, and run the md files through a html generator for netlify if netlify doesn't support serving md as html automatically (just looking into that now)?

tjenkinson · 2023-01-16T18:30:56Z

@robwalch I pushed 93244c5 to a branch which adds some validation to fail if the docs aren't in sync. I think with that it should be fine in the repo itself and less complicated than getting it built to html for netlify

With that LGTM!

tjenkinson · 2023-01-16T22:24:46Z

scripts/build-netlify.sh

@@ -10,8 +10,12 @@ echo "Building netlify..."

 # redirect / to /demo
 echo "/ /demo" > "$root/_redirects"
+
+# rediect /api-docs to github API.md
+echo "/api-docs https://github.com/video-dev/hls.js/tree/master/docs/API.md" > "$root/_redirects"


Just had a thought. It would be good to redirect to the commit specific url here it master so that old versions go to their correct docs

Can get the commit same way we do here

hls.js/scripts/deploy-netlify.sh

Line 6 in 0aa44f6

currentCommit=$(git rev-parse HEAD)

which should work when this runs in Netlify or actions

Can get the commit same way we do here

Hmm. Would be nice to see the version in the docs under Hls.version (but docs are generated from src not compiled js).

Looks good but I'm not sure about checking the generated markdown into the repo. There's currently nothing to validate that the docs are actually up to date so it's easy for them to go out of sync.

How about we keep them in the gitignore and build them on ci, and run the md files through a html generator for netlify if netlify doesn't support serving md as html automatically (just looking into that now)?

I would like to do this. Got it working with marked but the output is unstyled. Still looking into it.

I pushed 93244c5 to a branch which adds some validation to fail if the docs aren't in sync. I think with that it should be fine in the repo itself and less complicated than getting it built to html for netlify

Thanks for this. It is less complicated, but could lead to some confusion. Still looking into it.

markdown-styles works really nicely. After #5161 is in I'll rebased and check the netlify preveiw.

Yeh looks great

robwalch · 2023-01-17T05:01:15Z

package.json

@@ -27,15 +27,16 @@
    "build:watch": "webpack --progress --env debug --env demo --watch",
    "build:types": "tsc --build tsconfig-lib.json && api-extractor run --local",
    "dev": "webpack serve --progress --env debug --env demo --port 8000 --static .",
-    "docs": "esdoc",
+    "docs": "doctoc ./docs/API.md && api-documenter markdown -i api-extractor -o api-extractor/api-documenter && npm run docs-md-to-html",


Since doctoc ./docs/API.md modifies API.md should we still run a git diff --exit-code check on it?

Yeh I think it would be good to still do the check for api.md to make sure the contents is up to date

cherry-picked 93244c5 and removed the part that checked api-docs as that is no longer checked in as part of these changes.

itsjamie · 2023-01-17T16:34:57Z

Thanks! I'm glad I can archive those repos now 👍

…html

tjenkinson

Lgtm

Only comment is we should fix the versions in the package.json

tjenkinson · 2023-01-20T08:41:19Z

package.json

-    "@itsjamie/esdoc-ecmascript-proposal-plugin": "0.5.0",
-    "@itsjamie/esdoc-standard-plugin": "0.5.0",
-    "@itsjamie/esdoc-typescript-plugin": "0.5.0",
+    "@microsoft/api-documenter": "^7.19.27",


These should be fixed versions and then renovate work bump and test with new versions

robwalch requested review from itsjamie and tjenkinson January 16, 2023 03:58

robwalch added this to the 1.4.0 milestone Jan 16, 2023

robwalch changed the title ~~Replace esdocs generated api-docs with api-documenter markdown~~ Replace ESDocs generated api-docs with api-documenter markdown Jan 16, 2023

robwalch force-pushed the feature/variable-substitution branch from 53a339c to b8a9635 Compare January 16, 2023 05:58

robwalch force-pushed the task/api-docs-replace-esdocs-with-api-documenter-markdown branch from fed374e to 0aa44f6 Compare January 16, 2023 06:44

tjenkinson reviewed Jan 16, 2023

View reviewed changes

robwalch force-pushed the task/api-docs-replace-esdocs-with-api-documenter-markdown branch 3 times, most recently from db9316d to a1725b8 Compare January 17, 2023 04:49

robwalch commented Jan 17, 2023

View reviewed changes

itsjamie previously approved these changes Jan 17, 2023

View reviewed changes

robwalch force-pushed the feature/variable-substitution branch 2 times, most recently from a0d75e7 to f1996ef Compare January 18, 2023 22:55

robwalch force-pushed the task/api-docs-replace-esdocs-with-api-documenter-markdown branch from a1725b8 to 4f8d3ae Compare January 18, 2023 23:12

Base automatically changed from feature/variable-substitution to master January 20, 2023 00:52

robwalch dismissed itsjamie’s stale review via 4f8d3ae January 20, 2023 00:52

Replace ESDocs with API-Extractor > api-documenter > markdown-styles …

f25fa56

…html

robwalch force-pushed the task/api-docs-replace-esdocs-with-api-documenter-markdown branch from 4f8d3ae to f25fa56 Compare January 20, 2023 01:18

tjenkinson and others added 2 commits January 19, 2023 17:22

fail build if building docs results in a change

1ca8294

Only check API.md

a8565a4

robwalch requested a review from tjenkinson January 20, 2023 03:59

Update package lock

5a6338c

tjenkinson previously approved these changes Jan 20, 2023

View reviewed changes

Used fixed dependency versions

7a07f5c

robwalch dismissed tjenkinson’s stale review via 7a07f5c January 20, 2023 14:57

tjenkinson approved these changes Jan 20, 2023

View reviewed changes

robwalch merged commit 485fe15 into master Jan 20, 2023

robwalch deleted the task/api-docs-replace-esdocs-with-api-documenter-markdown branch January 20, 2023 16:57

robwalch added a commit that referenced this pull request Jan 26, 2023

Replace ESDocs generated api-docs with api-documenter markdown (#5163)

8740846

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace ESDocs generated api-docs with api-documenter markdown #5163

Replace ESDocs generated api-docs with api-documenter markdown #5163

robwalch commented Jan 16, 2023 •

edited

Loading

tjenkinson commented Jan 16, 2023

tjenkinson commented Jan 16, 2023

tjenkinson Jan 16, 2023

tjenkinson Jan 16, 2023

robwalch Jan 17, 2023 •

edited

Loading

robwalch Jan 17, 2023

tjenkinson Jan 17, 2023

robwalch Jan 17, 2023

tjenkinson Jan 17, 2023

robwalch Jan 20, 2023

itsjamie commented Jan 17, 2023

tjenkinson left a comment

tjenkinson Jan 20, 2023

robwalch Jan 20, 2023

Replace ESDocs generated api-docs with api-documenter markdown #5163

Replace ESDocs generated api-docs with api-documenter markdown #5163

Conversation

robwalch commented Jan 16, 2023 • edited Loading

This PR will...

Why is this Pull Request needed?

tjenkinson commented Jan 16, 2023

tjenkinson commented Jan 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robwalch Jan 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsjamie commented Jan 17, 2023

tjenkinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robwalch commented Jan 16, 2023 •

edited

Loading

robwalch Jan 17, 2023 •

edited

Loading