Improvise in-place-testnet docs #8805
Conversation
mattverse
added
V:state/compatible/no_backport
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request modifies the Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/localosmosis/README.md (2)
101-104: Add specific build command instruction.While adding the build prerequisite is helpful, it would be more user-friendly to include the specific command needed to build the binary.
Consider adding the actual build command, for example:
- On version X, build binary and run: + On version X, build the binary using `make build` and then run:
107-109: Fix typo and enhance version compatibility guidance.There's a typo in "exmaple" and the explanation could benefit from additional clarity about version compatibility.
Consider this revision:
- where vXX indicates the next version that mainnet needs to be upgraded to. For exmaple when current mainnet state is at v26, the flag value should be `--trigger-testnet-upgrade=v27`. + where vXX indicates the next version that mainnet needs to be upgraded to. For example, when current mainnet state is at v26, the flag value should be `--trigger-testnet-upgrade=v27`. Ensure that your local repository supports the target upgrade version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/localosmosis/README.md(2 hunks)
🧰 Additional context used
🪛 Markdownlint
tests/localosmosis/README.md
123-123: null
Bare URL used
(MD034, no-bare-urls)
123-123: null
Bare URL used
(MD034, no-bare-urls)
|
|
||
| The settings for in place testnet are done in https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L773. Modify the parameters in `InitOsmosisAppForTestnet` to modify in place testnet parameters. For example, if you were to modify epoch hours, you would be modifying https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L942-L967 . | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve documentation links for long-term maintainability.
The current implementation uses bare URLs and links to a specific commit hash, which could become outdated. Additionally, the links should be properly formatted for markdown.
Consider this revision:
-The settings for in place testnet are done in https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L773. Modify the parameters in `InitOsmosisAppForTestnet` to modify in place testnet parameters. For example, if you were to modify epoch hours, you would be modifying https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L942-L967 .
+The settings for in-place testnet are configured in the [`InitOsmosisAppForTestnet`](../../app/app.go) function. You can modify various testnet parameters in this function. For example, epoch hours can be adjusted in the `epochs` section of the same file.This change:
- Uses relative links that are more maintainable
- Removes dependency on specific line numbers that might change
- Follows proper markdown link syntax
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The settings for in place testnet are done in https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L773. Modify the parameters in `InitOsmosisAppForTestnet` to modify in place testnet parameters. For example, if you were to modify epoch hours, you would be modifying https://github.com/osmosis-labs/osmosis/blob/bb7a94e2561cc63b60ee76ec71a3e04e9688b22c/app/app.go#L942-L967 . | |
| The settings for in-place testnet are configured in the [`InitOsmosisAppForTestnet`](../../app/app.go) function. You can modify various testnet parameters in this function. For example, epoch hours can be adjusted in the `epochs` section of the same file. |
🧰 Tools
🪛 Markdownlint
123-123: null
Bare URL used
(MD034, no-bare-urls)
123-123: null
Bare URL used
(MD034, no-bare-urls)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tests/localosmosis/README.md (2)
101-109: Enhance clarity of build and upgrade instructions.The instructions could be more specific:
- Add the actual build command (
make buildor equivalent)- Add a comma after "For example"
- Consider providing a concrete example with actual version numbers for better clarity
Apply this diff:
-On version X, build binary and run: +On version X, build the binary using `make build` and then run: ```bash osmosisd in-place-testnet localosmosis osmo12smx2wdlyttvyzvzg54y2vnqwq2qjateuf7thj --trigger-testnet-upgrade=vXX-where vXX indicates the next version that mainnet needs to be upgraded to. For example when current mainnet state is at v26, the flag value should be
--trigger-testnet-upgrade=v27.
+where vXX indicates the next version that mainnet needs to be upgraded to. For example, if the current mainnet state is at v26 and you want to test the v27 upgrade, use--trigger-testnet-upgrade=v27.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [typographical] ~107-~107: After the expression ‘for example’ a comma is usually used. Context: ...at mainnet needs to be upgraded to. For example when current mainnet state is at v26, t... (COMMA_FOR_EXAMPLE) </details> </details> --- `122-131`: **Improve clarity of log noise reduction instructions.** The section could benefit from: 1. Adding a language specifier to the code block 2. Providing the exact content for the empty JSON file 3. Better formatting of the log example Apply these improvements: ```diff ### Removing unnecessary log lines Running in-place-testnet would be resulting in noise in the log lines that looks something like this. -``` +```json {"level": "info", "module": "server", "module": "consensus", "err": "cannot find validator 122 in valSet of size 1: invalid valid ator index", "time": "2024-10-21T21:34:30Z" ,"message": "failed attempting to add vote"} {"level": "error" , "module": "server", "module":" consensus" ,"height" :22731539, "round" :0, "peer": "27e14df66c9e4cd6b176b@dcabadfa 96750f911", "msg_type": "*consensus. VoteMessage", "err": "error adding vote", "time": "2024-10-21T21:34:307","message":"failed to process message"} {"level": "info" ,"module": "server" ', "module": "consensus", "err": "cannot find validator 107 in valSet of size 1: invalid valid ator index", "time": "2024-10-21T21:34:30Z", "message": "failed attempting to add vote"}-To remove the noise, remove existing addrbook.json from config folder, and replace it with an empty json.
+To remove the noise, remove the existingaddrbook.jsonfrom the config folder, and replace it with a new file containing an empty JSON object:
+
+json +{} +<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint</summary> 124-124: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a105f2a93930e4cea4dbc944d5d30f513e1021fc and f3f78bfdfccbfc62c606fc60ae2cfa2762172ac9. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `tests/localosmosis/README.md` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>tests/localosmosis/README.md</summary> [typographical] ~107-~107: After the expression ‘for example’ a comma is usually used. Context: ...at mainnet needs to be upgraded to. For example when current mainnet state is at v26, t... (COMMA_FOR_EXAMPLE) </details> </details> <details> <summary>🪛 Markdownlint</summary> <details> <summary>tests/localosmosis/README.md</summary> 134-134: null Bare URL used (MD034, no-bare-urls) --- 134-134: null Bare URL used (MD034, no-bare-urls) --- 124-124: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary> <details> <summary>tests/localosmosis/README.md (1)</summary> `132-135`: **Skip comment generation.** The existing review comment about improving documentation links is still valid and covers all necessary improvements. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint</summary> 134-134: null Bare URL used (MD034, no-bare-urls) --- 134-134: null Bare URL used (MD034, no-bare-urls) </details> </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Closes: #XXX
What is the purpose of the change
Improving in place testnet docs with information I found useful while I was testing and using in place testnet
Testing and Verifying
This change is a trivial rework / code cleanup without any test coverage.
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)