-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat: docs restructuring, add deploy section, bring in v0.14.x #493
Conversation
Remove GM tutorial, add language around choosing DA and Execution
Remove kurtosis from wordle tutorial. Rename wordle tutorial in menu to build your rollup. Create overview for deployments sections. Create docker compose deployment tutorial. Create kurtosis deployment tutorial. Refactor tutorials to reduce duplication.
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe changes in this pull request primarily involve modifications to the documentation structure in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (16)
tutorials/deploy-overview.md (4)
1-3
: Consider enhancing the frontmatter with additional metadata.The frontmatter could be enriched with additional fields to improve documentation organization and searchability.
Consider adding these fields:
--- description: This page provides an overview of some common ways to deploy rollups. +title: Deploying Your Rollup +sidebar_label: Overview +tags: ['deployment', 'tutorial', 'overview'] ---
11-13
: Enhance the transition to deployment methods.While the transition from helper scripts to production-ready deployments is clear, consider providing a brief overview of what makes these deployment methods more "production-ready" compared to helper scripts.
Add a brief explanation of the benefits, such as:
In the tutorials so far, you've seen various helper scripts used to make things easier. While great for tutorials, there are better ways to deploy and manage rollups than using various bash scripts. -In this section, you'll see a few examples of how you can deploy your rollup environment with all your services running in a more production-ready way. +In this section, you'll see a few examples of how you can deploy your rollup environment with all your services running in a more production-ready way. These methods offer benefits such as: + +* Reproducible deployments across different environments +* Better service orchestration and management +* Simplified configuration and maintenance +* Improved monitoring and logging capabilities
15-17
: Strengthen the security implications in the disclaimer.The current disclaimer is good but could better emphasize security considerations for production deployments.
Consider expanding the warning:
:::warning Disclaimer -These examples are for educational purposes only. Before deploying your rollup for production use you should fully understand the services you are deploying and your choice in deployment method. +These examples are for educational purposes only. Before deploying your rollup for production use: +* Fully understand the services you are deploying and your choice in deployment method +* Implement appropriate security measures and monitoring +* Consider the implications of each component choice on the overall security and reliability of your rollup +* Conduct thorough testing in a staging environment :::
19-20
: Add brief descriptions for each deployment method.Consider providing a quick overview of each deployment option to help readers choose the most appropriate method for their needs.
Enhance the links section:
-* [Deploy with Docker Compose](/tutorials/docker-compose) -* [Deploy with Kurtosis](/tutorials/kurtosis) +## Available Deployment Methods + +* [Deploy with Docker Compose](/tutorials/docker-compose) + * Suitable for local development and testing + * Uses Docker Compose for service orchestration + * Easier to customize and modify + +* [Deploy with Kurtosis](/tutorials/kurtosis) + * Provides a more automated deployment experience + * Includes built-in testing and verification + * Better suited for complex deploymentstutorials/docker-compose.md (3)
18-21
: Enhance prerequisites section with version requirements.Consider adding:
- Minimum Docker Compose version requirements
- System requirements (CPU, RAM, disk space)
- Operating system compatibility notes
186-213
: Enhance interaction documentation with troubleshooting guide.Consider adding:
- Common error scenarios and their solutions
- Troubleshooting steps for connection issues
- Backup and restore procedures for the rollup data
- Instructions for viewing logs (
docker logs wordle
)
222-224
: Add cleanup and production deployment sections.Consider adding:
- Cleanup instructions (
docker compose down -v
)- Production deployment considerations:
- Security hardening
- Monitoring and logging
- Backup strategies
- High availability setup
tutorials/kurtosis.md (6)
7-9
: Improve grammar in the disclaimer messageThe warning message could be clearer with better punctuation and structure.
Consider this revision:
-Kurtosis currently does not fully support data persistence across runs, because of this it is not recommended for production use. +Kurtosis currently does not fully support data persistence across runs; therefore, it is not recommended for production use.🧰 Tools
🪛 LanguageTool
[typographical] ~8-~8: It appears that a comma is missing.
Context: ...ata persistence across runs, because of this it is not recommended for production us...(BECAUSE_OF_YOU_COMMA)
22-42
: Enhance prerequisites sectionConsider adding more specific prerequisites:
- Required software versions (Docker, etc.)
- System requirements
- Expected time to complete
Also, consider making the CLI version output more generic to avoid maintenance overhead when new versions are released.
75-78
: Add validation step for Docker imageConsider adding a step to verify the image functionality:
# Verify the image was built correctly docker run --rm wordle rollkit version
102-141
: Enhance Kurtosis configuration documentationThe configuration could benefit from:
- Comments explaining each parameter's purpose
- Error handling guidance
- Common troubleshooting scenarios
Consider adding comments to the Python code:
def run(plan): + # Start the DA node and get its address + # The DA node is required for the rollup to submit blocks da_address = da_node.run( plan, )
155-258
: Simplify example outputsConsider shortening the example outputs to show only the relevant parts and add comments explaining what to look for. This will make the tutorial more maintainable and easier to follow.
203-267
: Add troubleshooting sectionConsider adding a troubleshooting section that covers common issues users might encounter when:
- Connecting to the rollup
- Executing commands inside the container
- Handling network connectivity issues
tutorials/wordle.md (3)
33-36
: Installation instructions should specify version requirements.While the installation command includes a version tag, it would be helpful to specify the minimum required versions of Go and other dependencies upfront.
Consider adding a requirements section:
### Requirements - Go 1.21 or later - Rollkit CLI {{constants.rollkitLatestTag}} or later
Line range hint
214-276
: Consider adding validation explanations for scaffold commands.The scaffold commands are correct, but it would be helpful to explain the validation rules that will be implemented (e.g., 5-letter word, alphabets only) before showing the keeper implementation.
589-589
: Add transaction response handling guidance.The tutorial shows how to submit transactions but should explain how to handle common error scenarios and transaction failures.
Add a troubleshooting section:
### Troubleshooting Transaction Errors - `timeout`: Increase the timeout value or retry with async flag - `out of gas`: Adjust the gas limit - `insufficient funds`: Ensure your account has enough tokensAlso applies to: 696-696, 708-708, 720-720
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- .vitepress/config.ts (3 hunks)
- tutorials/deploy-overview.md (1 hunks)
- tutorials/docker-compose.md (1 hunks)
- tutorials/kurtosis.md (1 hunks)
- tutorials/wordle.md (11 hunks)
🧰 Additional context used
🪛 Markdownlint
tutorials/docker-compose.md
57-57: Column: 1
Hard tabs(MD010, no-hard-tabs)
58-58: Column: 1
Hard tabs(MD010, no-hard-tabs)
59-59: Column: 1
Hard tabs(MD010, no-hard-tabs)
60-60: Column: 1
Hard tabs(MD010, no-hard-tabs)
93-93: Column: 1
Hard tabs(MD010, no-hard-tabs)
94-94: Column: 1
Hard tabs(MD010, no-hard-tabs)
🪛 LanguageTool
tutorials/kurtosis.md
[typographical] ~8-~8: It appears that a comma is missing.
Context: ...ata persistence across runs, because of this it is not recommended for production us...(BECAUSE_OF_YOU_COMMA)
tutorials/wordle.md
[style] ~78-~78: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...--> ### 🔥 Ignite {#ignite} Ignite is an amazing CLI tool to help us get started buildin...(AWESOME)
[uncategorized] ~92-~92: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ocal machine. This tutorial uses a macOS but it should work for Windows. For Windows...(COMMA_COMPOUND_SENTENCE)
[grammar] ~716-~716: When ‘five-letter’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... for the day. Now let’s try to guess a five letter word: <!-- markdownlint-disable MD013 ...(WORD_ESSAY_HYPHEN)
🔇 Additional comments (6)
.vitepress/config.ts (3)
Line range hint
221-275
: Documentation structure changes look good!The navigation restructuring aligns well with the PR objectives. The new organization with "Build a Rollup", updated execution options, and the new deployment section provides a clear, logical flow for users.
341-359
: Integration section is well-structuredThe new integrations are logically organized and provide clear paths for users to implement various features, from smart contract deployment to monitoring and token configuration.
Line range hint
221-275
: Verify removal of old sectionsThe new structure looks good, but let's verify that the old "Rollkit" section and GM tutorial have been properly removed.
tutorials/kurtosis.md (1)
1-282
: Overall assessment: Well-structured tutorial with room for enhancementThe tutorial successfully guides users through deploying a Wordle rollup using Kurtosis. The content is logically organized and covers the complete workflow. To further improve it, consider:
- Making it more self-contained by including direct content rather than references
- Adding comprehensive troubleshooting guidance
- Enhancing validation steps
- Simplifying example outputs for better maintainability
🧰 Tools
🪛 LanguageTool
[typographical] ~8-~8: It appears that a comma is missing.
Context: ...ata persistence across runs, because of this it is not recommended for production us...(BECAUSE_OF_YOU_COMMA)
tutorials/wordle.md (2)
73-107
: LGTM: Clear module scaffolding explanation.The section effectively explains the module structure and its integration with Ignite CLI.
🧰 Tools
🪛 LanguageTool
[style] ~78-~78: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...--> ### 🔥 Ignite {#ignite} Ignite is an amazing CLI tool to help us get started buildin...(AWESOME)
[uncategorized] ~92-~92: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ocal machine. This tutorial uses a macOS but it should work for Windows. For Windows...(COMMA_COMPOUND_SENTENCE)
546-581
: Verify local DA node requirements.The tutorial introduces local DA node setup but doesn't specify its system requirements or potential conflicts with other services on port 7980.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside from some nits, left them as suggestions
@coderabbitai ignore |
@coderabbitai ignore ✅ Actions performedReviews paused. |
@coderabbitai resolve |
✅ Actions performedComments resolved. |
Overview
This PR handles the major changes outline in #480
These changes include:
Rollkit
section and GM tutorialBuild Your Rollup
Deploy Your Rollup
section with docker composeBuilding on top of #497
Closes: #500