Skip to content

Comments

chore: Improve Documentation#690

Merged
mergify[bot] merged 10 commits intosigp:unstablefrom
AgeManning:doc-improvement
Oct 17, 2025
Merged

chore: Improve Documentation#690
mergify[bot] merged 10 commits intosigp:unstablefrom
AgeManning:doc-improvement

Conversation

@AgeManning
Copy link
Member

This improves the anchor documentation (at least I hope its an improvement).

I've modified some of the example commands, updated the CLI reference guide, added a migrating to anchor section as well as a Running a Validator on SSV section.

I've modified the main landing page, to feature switching to anchor. I am aware that the landing page says switching and the docs say migrating. I liked switching on the landing, happy to change though for consistency.

I removed an obsolete CLI check, that I believe is now doing nothing.

This PR probably does need some level of eye-balling to make sure the changes are sound. Especially around the migrating, as I didn't actually test that.

Future Work

I think the FAQ section might need some love. Maybe there have been some questions on discord that might be useful cc @chong-he

@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Oct 15, 2025

Claude finished @AgeManning's task —— View job


Documentation Review Complete

Tasks

  • Read repository guidelines (CLAUDE.md)
  • Review workflow changes (.github/workflows/test-suite.yml)
  • Review documentation structure changes (vocs.config.ts)
  • Review CLI reference updates (cli*.mdx files)
  • Review new migration guide
  • Review new "Running a Validator on SSV" guide
  • Review landing page and introduction changes
  • Verify CLI command accuracy against codebase
  • Check for consistency issues
  • Provide comprehensive feedback

Overall Assessment

This documentation PR improves the structure and usability of the Anchor documentation. The additions of migration guide and validator setup instructions are valuable. The removal of the obsolete CLI check from workflows is appropriate.

Issues Found

Critical Issues

1. Broken Link in Landing Page (docs/docs/pages/index.mdx:23)

The landing page links to /switch_to_anchor but the actual page is named migrate_to_anchor.mdx:

<a href="/switch_to_anchor" className="btn btn-secondary">Switch to Anchor</a>

This needs to be changed to /migrate_to_anchor to match the actual filename.

2. Inconsistent Link Reference (docs/docs/pages/running_an_operator.mdx:7)

References /switching_to_anchor (also incorrect):

[switching to anchor](/switching_to_anchor)

Should be /migrate_to_anchor.

Documentation Accuracy Issues

3. Incorrect Flag in CLI Example (docs/docs/pages/running_an_operator.mdx:92)

The example uses --datadir but this flag doesn't exist. Based on the CLI code, it should be --data-dir:

# Current (incorrect)
anchor node \
  --datadir ~/.anchor/hoodi \

# Should be
anchor node \
  --data-dir ~/.anchor/hoodi \

Actually, looking at the text above (line 98), you mention the flag can be omitted since it uses default values, so this could just be removed from the example entirely.

4. Missing --output-path Flag in keygen Example (docs/docs/pages/running_an_operator.mdx:55)

The example shows:

anchor keygen --encrypt --output-path /path/to/keys/directory

However, based on the keygen code (anchor/keygen/src/lib.rs), there is no --output-path flag. The tool uses --data-dir to specify the output location. This should be:

anchor keygen --encrypt --data-dir /path/to/keys/directory

5. Inconsistent Terminology

As you noted, the landing page uses "Switch to Anchor" while the documentation uses "Migrate to Anchor". For consistency:

  • Either rename the page to switch_to_anchor.mdx and update the sidebar
  • Or change the landing page button text to "Migrate to Anchor"

I suggest keeping "Migrate to Anchor" throughout as it's more technically accurate for the operation being performed.

Minor Issues

6. Version Number Inconsistency (docs/docs/pages/installation.mdx:38)

The example shows version v0.3.1 which is outdated. Should use v1.0.0-rc.1 to match the current release mentioned elsewhere in the docs.

7. Typo in Landing Page (docs/docs/pages/index.mdx:107)

"reguarly" should be "regularly":

<p>We reguarly fuzz both the Anchor and the go-ssv client

8. Grammar Issue (docs/docs/pages/introduction.mdx:22)

Double "is":

Anchor specifically is is optimized for efficiency

Should be:

Anchor is specifically optimized for efficiency

9. Missing Network Option in keysplit Example (docs/docs/pages/cli-keysplit.mdx:58)

The manual keysplit example doesn't include the --network flag, but the onchain example does (line 79). For consistency and clarity, consider adding it to the manual example as well, or add a note that it defaults to hoodi.

Positive Aspects

  1. Well-structured migration guide - Clear step-by-step instructions with appropriate warnings about running duplicate operators
  2. Good addition of client dependencies section - Helps operators understand the full setup requirements
  3. Appropriate removal of obsolete CI check - The commented-out cli-check job was properly removed
  4. Improved CLI documentation - The CLI reference pages are well-organized with clear tables and examples
  5. Helpful info blocks - Good use of info/warning callouts (e.g., the reth pruning warning)

Recommendations

  1. Test the migration instructions - Since you mentioned not testing the migration steps, I recommend having someone verify the exact commands work as documented
  2. Verify all internal links - Run through all documentation links to ensure they resolve correctly
  3. Consider adding a troubleshooting section - Common issues when migrating (e.g., permission errors, wrong file locations) would be helpful
  4. Update version numbers - Ensure all version references point to v1.0.0-rc.1 or use placeholders like <version>

Summary

The documentation improvements are valuable, but there are several critical link errors and CLI flag inaccuracies that need to be fixed before merging. The broken links will result in 404 errors for users, and the incorrect CLI flags will cause confusion and failed commands.

Priority fixes needed:

  1. Fix all /switch_to_anchor and /switching_to_anchor links → /migrate_to_anchor
  2. Remove or correct the --datadir flag
  3. Change --output-path to --data-dir in keygen example
  4. Fix typos: "reguarly" → "regularly", "is is" → "is"
  5. Update version numbers in examples

@dknopik
Copy link
Member

dknopik commented Oct 15, 2025

@claude Isn't --datadir an alias for --data-dir?

@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Oct 15, 2025

Claude finished @dknopik's task —— View job


Investigating --datadir Flag

Tasks

  • Search for CLI argument definitions in the codebase
  • Verify if --datadir is an alias for --data-dir
  • Update review feedback with correct information

Findings

You're absolutely right! I apologize for the error in my previous review.

I found the flag definition in anchor/common/global_config/src/lib.rs:25-36:

#[clap(
    long,
    short = 'd',
    global = true,
    value_name = "DIR",
    help = "Used to specify a custom root data directory for the Anchor key and database. \
            Defaults to $HOME/.anchor/{network} where network is the value of the `network` flag \
            Note: Users should specify separate custom datadirs for different networks.",
    display_order = 0,
    alias = "datadir"  // <-- HERE
)]
pub data_dir: Option<PathBuf>,

--datadir IS a valid alias for --data-dir, defined at line 34 with alias = "datadir".

Corrected Review

The example in docs/docs/pages/running_an_operator.mdx:92 using --datadir is correct and does not need to be changed. Both --data-dir and --datadir are valid flags.

However, the other issues I identified in my original review still stand:

  • Critical: Broken links (/switch_to_anchor and /switching_to_anchor should be /migrate_to_anchor)
  • Issue: The keygen --output-path flag doesn't exist (should be --data-dir)
  • Minor typos and grammar issues

I apologize for the confusion on this point. Thank you for catching this!


View job | Branch: doc-improvement

@dknopik
Copy link
Member

dknopik commented Oct 15, 2025

@claude can you please implement your remaining suggestions? If necessary, open a PR against Age's branch.

@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Oct 15, 2025

Claude finished @dknopik's task —— View job


Implementing Documentation Fixes

Tasks

  • Read CLAUDE.md and affected files
  • Fix broken links in index.mdx (/switch_to_anchor/migrate_to_anchor)
  • Fix broken link in running_an_operator.mdx (/switching_to_anchor/migrate_to_anchor)
  • Fix keygen --output-path flag in running_an_operator.mdx (should be --data-dir)
  • Fix typo "reguarly" → "regularly" in index.mdx
  • Fix grammar "is is" → "is" in introduction.mdx
  • Update version numbers to v1.0.0-rc.1 where needed
  • Run markdown linting if possible
  • Commit and push changes

View job | Branch: doc-improvement

@dknopik
Copy link
Member

dknopik commented Oct 15, 2025

I think I bricked it ;(

@jking-aus
Copy link
Member

I think I bricked it ;(

I don't think he has perms for new branch PRS. I will look into it

@dknopik
Copy link
Member

dknopik commented Oct 15, 2025

@jking-aus nah I just wanted to try it :) definitely not urgent. I bet I could instruct my local claude to read the comment and perform the edits, which is probably more reasonable than me vibecoding though the github comment section

@AgeManning
Copy link
Member Author

@claude Can you review the current state of the PR now. Is it up to your standards?

@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Oct 16, 2025

Claude finished @AgeManning's task —— View job


Reviewing Current PR State

Tasks

  • Read all changed documentation files
  • Check for previously identified issues
  • Verify link consistency
  • Check CLI flag accuracy
  • Look for typos and grammar issues
  • Provide comprehensive assessment

View job | Branch: doc-improvement

@chong-he
Copy link
Member

There isn't much questions on discord about Anchor yet, but I will think of something to put in the faqs

# Specific version example
https://github.com/sigp/anchor/releases/download/v0.3.1/anchor-v0.3.1-x86_64-unknown-linux-gnu.tar.gz
tar -xvf anchor-v0.3.1-x86_64-unknown-linux-gnu.tar.gz
wget https://github.com/sigp/anchor/releases/download/v1.0.0-rc.1/anchor-v1.0.0-rc.1-x86_64-unknown-linux-gnu.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure if our version update script can properly replace the -rc.1, it looks like the regex is just replacing the part consisting of digits and dots?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should either put 0.0.0 so that it is obvious that this is to be replaced, or put some obvious placeholder like ##VERSION##

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah nice catch. The regex didn't include the rc versions. I've improved the regex so that it does.

That script didn't include this file however. This file had hard-coded examples. Figure might as well update this, so changed the script to include this file.

Of course with scripts like this, if the filename changes or format changes it will break. But I think the main page probably will most likely be static and if this page changes, I think its fine to have an out-of-date example.

Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM

@mergify mergify bot merged commit f4ad59a into sigp:unstable Oct 17, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants