chore: fix documentation CLI #6566

jeluard · 2024-03-19T13:51:31Z

Motivation

Make sure documentation CLI properly allow access to command arguments.

Note: format changes are the result of yarn docs:lint:fix

jeluard · 2024-03-20T07:39:53Z

.github/workflows/docs-check.yml

@@ -38,14 +38,13 @@ jobs:
      - name: Check wordlist is sorted
        run: scripts/wordlist_sort_check.sh

+      - name: Spellcheck


Moved before the CLI reference generation as otherwise spellcheck fails on all custom headers ids.
We use custom header ids for subcommand arguments (- instead of --) so that previous links are not broken.

CLI docs are already checked as they are part of the source code.

CLI docs are already checked as they are part of the source code.

What do you mean here? I don't think we are spellchecking the source code

Hmm that's unfortunate.

Spellchecking the CLIs docs fail because spellcheck wants to validate all ids. Not providing custom ids would potentially break external links.
Apparently there is no alternative with latest docusaurus.

The solution probably is to switch to mdx and explicitly export TOC metadata via the toc global. See facebook/docusaurus#6201 (comment)

so before did it pass validation because we used backticks `` for those values, and can't we use this approach as well or does it not format as we want it to?

Unfortunately spellcheck is also applied to the custom id part, and this part can't be escaped.
e.g. ### `--myArg` {#-myArg}

Fixed it by programmatically generating the TOC. This open the door for more customizations down the line.

Do you have these changes deployed somewhere to review? I guess I can run the branch locally as well to review the rendered UI. This is similar to ethereum/beacon-APIs#415 would be great to have but likely a bit of effort.

But since we plan to add more complex / interactive code examples in the docs, it might be warranted to implement as it makes reviewing so much easier

You can see it deployed here: https://jeluard.github.io/lodestar/validator-management/validator-cli

codecov · 2024-03-21T10:42:58Z

Codecov Report

Merging #6566 (ecaee03) into unstable (da69a7b) will increase coverage by 0.00%.
Report is 4 commits behind head on unstable.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #6566   +/-   ##
=========================================
  Coverage     61.48%   61.49%           
=========================================
  Files           556      556           
  Lines         58891    58895    +4     
  Branches       1854     1856    +2     
=========================================
+ Hits          36212    36216    +4     
  Misses        22638    22638           
  Partials         41       41

nflaig · 2024-03-25T11:12:05Z

docs/package.json

-    "@docusaurus/core": "3.1.1",
-    "@docusaurus/preset-classic": "3.1.1",
-    "@docusaurus/theme-mermaid": "^3.1.1",
+    "@docusaurus/core": "0.0.0-5872",


Do we wanna wait until this fix is in a stable release? Deadline would be the 1.18.0 if it is not released by then we can go ahead and merge this

I would lean towards merging, as it would allow to work on things like optimizing SEO in parallel.
Given that this dependency is docs specifics, I think it's fine.

nflaig · 2024-03-25T11:14:23Z

packages/light-client/README.md

 });

 lightclient.emitter.on(LightclientEvent.lightClientOptimisticHeader, async (optimisticUpdate) => {
-    logger.info(optimisticUpdate);


How did you find all these formatting issues, just manual review?

Those were identified by yarn docs:lint

ah right sorry, you already noted that in the PR description but I am a bit confused, how was this not caught by the CI

lodestar/.github/workflows/docs-check.yml

Lines 46 to 47 in 533a08e

- name: Check docs format

run: yarn docs:lint:fix

This line actually fixes the format directly without failing, then the workflow proceeds.

This line actually fixes the format directly without failing, then the workflow proceeds.

this seems wrong, at least in .github/workflows/docs-check.yml it doesn't seem right to run lint:fix, should just do lint check instead

Makes sense, changed so that it runs yarn docs:lint

Makes sense, changed so that it runs yarn docs:lint

Would be good to double check that it now also fails if there is a lint issue. Ideally, we should not have this change in this PR (Im ok with having it here tho)

It does, I had to fix an extra lint issue :)

nflaig

Noticed that URLs are broken again on this branch

https://jeluard.github.io/lodestar/validator-management/validator-cli/#-keymanager.port

vs.

https://chainsafe.github.io/lodestar/validator-management/validator-cli/#-keymanagerport

nflaig · 2024-03-25T12:57:23Z

packages/cli/docsgen/markdown.ts

@@ -90,7 +90,7 @@ function renderExamplesSection(examples: CliExample[], sectionTitle?: string, lo
 function renderOption(optionName: string, option: CliOptionDefinition): string | undefined {
  if (option.hidden) return;

-  const commandOption = [`<h3 id='-${optionName}'><code>--${optionName}</code><a href="#-${optionName}" class="hash-link" title="--${optionName}"></a></h3>`];
+  const commandOption = [`### \`--${optionName}\` {#-${optionName}}`];


The cli flag headers look much bigger than what we currently have

looks like we add 4x # previously

lodestar/packages/cli/docsgen/markdown.ts

Line 89 in c3afb0b

const commandOption = [`#### \`--${optionName}\``];

jeluard · 2024-03-25T13:06:13Z

Actually I realize that my solution of generating the TOC doesn't help with the spell-check issue, as there is still a need to generate the html anchor using custom-ids.

So we are back to the same 2 options:

do not spell-check generated CLI docs
have links pointing to CLI args broken (- vs --)

Note that there is a way to handle link redirects with docusaurus, so we could handle if the number of broken links is not too big.

My preference would be to bite the bullet and fix links at the source as much as we can and adopt the new link format.

nflaig · 2024-03-25T13:06:26Z

Some nav links from toc on the sidebar don't work

Results in this URL: https://jeluard.github.io/lodestar/validator-management/validator-cli/#slashing-protection%20%3Ccommand%3E

nflaig · 2024-03-25T13:11:06Z

My preference would be to bite the bullet and fix links at the source as much as we can and adopt the new link format.

The majority of broken links were actually due to splitting docs up and moving into different sub paths. Since linking to specific CLI flag is quite new, the amount of broken links will be limited, and worst case the user still ends up on the right CLI docs page, just not navigated to the flag directly.

Let's go ahead and use the approach that works with our new docs setup the best and without workarounds, I think I know pretty well were to fix docs, and will do that once we release the new docs

This reverts commit 652422a.

jeluard · 2024-03-25T13:48:28Z

Latest version using the out-of-the-box docusaurus solution is deployed. Sounds like a more robust approach.

nflaig

LGTM, I think before we do the release we should do another sweep and compare the new docs vs currently deployed docs

jeluard · 2024-03-25T15:05:35Z

I deployed unstable here: https://jeluard.github.io/lodestar/
I did some sanity checks, looks good to me.

nflaig · 2024-03-25T16:17:12Z

I deployed unstable here: https://jeluard.github.io/lodestar/ I did some sanity checks, looks good to me.

Yes looks mostly correct, I opened a PR with some minor fixes #6590. Also gave this a try locally, devex is very good now :)

wemeetagain · 2024-04-23T19:04:56Z

🎉 This PR is included in v1.18.0 🎉

jeluard marked this pull request as ready for review March 19, 2024 20:56

jeluard requested a review from a team as a code owner March 19, 2024 20:56

jeluard commented Mar 20, 2024

View reviewed changes

jeluard added 5 commits March 21, 2024 11:28

chore: fix documentation CLI

a3c0963

chore: lint md files

1595125

chore: do not run spellcheck on generated CLI md

fc55773

chore: address comments

4e90d79

chore: address comments

2c3b4f1

jeluard mentioned this pull request Mar 21, 2024

Migrate to docusaurus #6517

Closed

8 tasks

jeluard added 2 commits March 23, 2024 09:45

chore: escape titles

0c2bebb

chore: generate TOC

652422a

nflaig reviewed Mar 25, 2024

View reviewed changes

chore: address comments

f5db159

chore: fix lint

6a0f10f

jeluard added 2 commits March 25, 2024 14:18

Revert "chore: generate TOC"

769ab15

This reverts commit 652422a.

chore: remove custom ids and move down header

ecaee03

nflaig approved these changes Mar 25, 2024

View reviewed changes

jeluard merged commit 948de61 into ChainSafe:unstable Mar 25, 2024
18 of 20 checks passed

jeluard deleted the jeluard/fix-documentation-cli branch March 25, 2024 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: fix documentation CLI #6566

chore: fix documentation CLI #6566

jeluard commented Mar 19, 2024 •

edited

Loading

jeluard Mar 20, 2024 •

edited

Loading

nflaig Mar 20, 2024

jeluard Mar 21, 2024 •

edited

Loading

jeluard Mar 21, 2024

nflaig Mar 21, 2024

jeluard Mar 23, 2024

jeluard Mar 25, 2024

nflaig Mar 25, 2024

nflaig Mar 25, 2024

jeluard Mar 25, 2024

codecov bot commented Mar 21, 2024 •

edited

Loading

nflaig Mar 25, 2024

jeluard Mar 25, 2024

nflaig Mar 25, 2024

jeluard Mar 25, 2024

nflaig Mar 25, 2024

jeluard Mar 25, 2024

nflaig Mar 25, 2024 •

edited

Loading

jeluard Mar 25, 2024

nflaig Mar 25, 2024 •

edited

Loading

jeluard Mar 25, 2024

nflaig left a comment •

edited

Loading

nflaig Mar 25, 2024

nflaig Mar 25, 2024

jeluard commented Mar 25, 2024

nflaig commented Mar 25, 2024

nflaig commented Mar 25, 2024

jeluard commented Mar 25, 2024

nflaig left a comment

jeluard commented Mar 25, 2024

nflaig commented Mar 25, 2024

wemeetagain commented Apr 23, 2024

chore: fix documentation CLI #6566

chore: fix documentation CLI #6566

Conversation

jeluard commented Mar 19, 2024 • edited Loading

jeluard Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeluard Mar 21, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 21, 2024 • edited Loading

Codecov Report

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

nflaig Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nflaig Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nflaig left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeluard commented Mar 25, 2024

nflaig commented Mar 25, 2024

nflaig commented Mar 25, 2024

jeluard commented Mar 25, 2024

nflaig left a comment

Choose a reason for hiding this comment

jeluard commented Mar 25, 2024

nflaig commented Mar 25, 2024

wemeetagain commented Apr 23, 2024

jeluard commented Mar 19, 2024 •

edited

Loading

jeluard Mar 20, 2024 •

edited

Loading

jeluard Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading

nflaig Mar 25, 2024 •

edited

Loading

nflaig Mar 25, 2024 •

edited

Loading

nflaig left a comment •

edited

Loading