-
Notifications
You must be signed in to change notification settings - Fork 52
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
chore: remove gm from some guides #491
Conversation
WalkthroughThe pull request includes updates to two documentation files: Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 1
🧹 Outside diff range and nitpick comments (5)
guides/block-times.md (2)
3-5
: Approved: Clear improvement in prerequisites and navigation.The changes effectively remove the reference to "GM world" and replace it with a link to the quick start tutorial, aligning with the PR objectives. This update improves navigation within the documentation and clearly states the prerequisites for experimenting with faster block times.
Consider adding a brief explanation of why these prerequisites are necessary for experimenting with faster block times. This could help users understand the importance of completing these steps before proceeding.
Line range hint
1-41
: Approved: Clear and concise guide for changing block production speed.The document provides a clear and concise guide for changing the speed of block production in Rollkit. The examples are varied and helpful, covering different scenarios from 100 milliseconds to 30 seconds.
To further improve the document:
- Consider adding a brief explanation of the potential impacts or use cases for different block times. This could help users understand when and why they might want to adjust the block time.
- You might want to include a note about any limitations or considerations when setting very short block times (e.g., 100ms). Are there any potential drawbacks or system requirements that users should be aware of?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: # How to change speed of block production If you have gone t...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
guides/restart-rollup.md (3)
9-31
: Excellent improvements to clarity and user guidance!The changes in this section significantly enhance the guide's usability:
- The simplified wording makes the instructions more straightforward.
- The addition of the quick start tutorial example provides context for new users.
- The inclusion of example output helps users understand what to expect.
These modifications will greatly assist users in restarting their rollups.
There's a minor grammatical error on line 18. Consider changing "You would have see output similar to:" to "You would have seen output similar to:"
🧰 Tools
🪛 LanguageTool
[grammar] ~18-~18: The verb form ‘see’ does not seem to be suitable in this context.
Context: ...`bash rollkit startYou would have see output similar to:
bash I[2024-10-1...(HAVE_VB)
[style] ~27-~27: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... module=txindex height=7 ``` If you need to restart your rollup, you can run the sa...(REP_NEED_TO_VB)
33-42
: Great additions for user understanding and proper rollup restart!The changes in this section provide valuable information:
- The explanation about block height continuation helps users understand the restart process.
- The example output reinforces the explanation visually.
- The note about including additional flags is crucial for ensuring a proper rollup restart.
These additions significantly improve the guide's completeness and user-friendliness.
To improve clarity, consider rephrasing the last sentence on line 42 to:
"For example, if you used the--rollkit.da_namespace
flag when first starting your rollup, you must include that same flag when restarting to ensure your rollup continues to publish blobs to the same namespace."🧰 Tools
🪛 LanguageTool
[style] ~42-~42: This phrase is redundant. Consider writing “started”.
Context: ...additional flags that you used when you first started your rollup. For example, if you used t...(FIRST_BEGAN)
91-92
: Effective deduplication and improved guide structureThe update to the gas fee section is a positive change:
- It helps deduplicate information by referring to a separate guide for detailed instructions.
- This approach maintains a more focused and modular documentation structure.
To improve clarity, consider adding a brief explanation of why increasing the gas fee is necessary. For example:
"To reiterate, before restarting the chain, you will need to increase the gas fee to override the transaction stuck in the mempool and avoid aCode: 19
error. See the How to configure gas price guide for more information."🧰 Tools
🪛 LanguageTool
[style] ~91-~91: Consider a shorter alternative to avoid wordiness.
Context: ..., you will need to increase the gas fee in order to avoid aCode: 19
error. See the [How ...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- guides/block-times.md (1 hunks)
- guides/restart-rollup.md (3 hunks)
🧰 Additional context used
🪛 LanguageTool
guides/restart-rollup.md
[grammar] ~18-~18: The verb form ‘see’ does not seem to be suitable in this context.
Context: ...`bash rollkit startYou would have see output similar to:
bash I[2024-10-1...(HAVE_VB)
[style] ~27-~27: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... module=txindex height=7 ``` If you need to restart your rollup, you can run the sa...(REP_NEED_TO_VB)
[style] ~42-~42: This phrase is redundant. Consider writing “started”.
Context: ...additional flags that you used when you first started your rollup. For example, if you used t...(FIRST_BEGAN)
[style] ~91-~91: Consider a shorter alternative to avoid wordiness.
Context: ..., you will need to increase the gas fee in order to avoid aCode: 19
error. See the [How ...(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (3)
guides/restart-rollup.md (3)
51-51
: Appropriate update to the tutorial referenceThe replacement of the "GM world" reference with a link to the quick start tutorial is a good change. It aligns with the PR objective of removing GM tutorial references and provides users with a more current and general guide.
87-87
: Improved generalization of rollup stopping instructionsThe simplification of the rollup stopping instruction is a positive change. By removing the specific reference to the "gm chain," the guide becomes more universally applicable to all rollups. This modification aligns well with the PR's objective of streamlining the documentation.
Line range hint
1-100
: Overall excellent improvements to the guideThe changes made to this guide successfully achieve the PR objectives and significantly enhance its quality:
- References to the GM tutorial have been removed and replaced with more general, up-to-date information.
- Information has been deduplicated by referring to separate guides for detailed instructions.
- The guide has been made more clear, concise, and user-friendly through improved wording and structure.
- Additional examples and explanations have been added to improve user understanding of the rollup restart process.
These modifications will greatly benefit users trying to restart their Rollkit rollups. The guide now provides a more streamlined, focused, and comprehensive set of instructions.
Great job on these improvements!
🧰 Tools
🪛 LanguageTool
[style] ~8-~8: Consider a shorter alternative to avoid wordiness.
Context: ...where you need to restart your rollup. In order to restart your rollup, you simply need to...(IN_ORDER_TO_PREMIUM)
[grammar] ~18-~18: The verb form ‘see’ does not seem to be suitable in this context.
Context: ...`bash rollkit startYou would have see output similar to:
bash I[2024-10-1...(HAVE_VB)
[style] ~27-~27: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... module=txindex height=7 ``` If you need to restart your rollup, you can run the sa...(REP_NEED_TO_VB)
[style] ~42-~42: This phrase is redundant. Consider writing “started”.
Context: ...additional flags that you used when you first started your rollup. For example, if you used t...(FIRST_BEGAN)
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.
gn, gm
Overview
This PR removes reference to the GM tutorial in the restart your rollup how to guide. It also deduplicates some commands by referencing other sections and guides.
It also removes it from the block times guide.
part of #485
Summary by CodeRabbit