docs: fix pre.sh #22194
docs: fix pre.sh #22194
Conversation
julienrbrt
added
the
backport/v0.52.x
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request modifies the Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
docs/pre.sh (1)
Line range hint
20-46: LGTM! Consider using variables for repeated paths.The documentation copying section looks good and is consistent with the new
build/directory structure. All necessary documentation seems to be properly copied to their respective locations.To improve maintainability, consider using variables for frequently used paths. For example:
BUILD_DIR="./build" MODULES_DIR="${BUILD_DIR}/modules" TOOLING_DIR="${BUILD_DIR}/tooling" PACKAGES_DIR="${BUILD_DIR}/packages" # Then use these variables in your copy commands cp ../tools/cosmovisor/README.md "${TOOLING_DIR}/01-cosmovisor.md" cp ../depinject/README.md "${PACKAGES_DIR}/01-depinject.md" # ... and so onThis approach would make it easier to update paths in the future if needed.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/pre.sh (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
docs/pre.sh (2)
Line range hint
20-21: Address TODO comment and consider adding error handling.There's a TODO comment regarding the vesting submodule that references an issue:
## TODO to be removed in https://github.com/cosmos/cosmos-sdk/issues/9958Please check the status of this issue and consider addressing it if possible. Run the following script to verify the issue status:
#!/bin/bash # Description: Check the status of the referenced GitHub issue gh issue view 9958 --repo cosmos/cosmos-sdkAdditionally, consider adding basic error handling and logging to the script. For example:
set -e # Exit immediately if a command exits with a non-zero status set -u # Treat unset variables as an error log_error() { echo "ERROR: $1" >&2 } # Use this function for critical operations, e.g.: # cp ../x/auth/vesting/README.md ./build/modules/auth/1-vesting.md || log_error "Failed to copy vesting README"This will make the script more robust and easier to debug if issues arise.
10-10: Verify the intentionality of the documentation path change.The
MODULE_DIRECTORYpath has been changed fromdocs/build/modules/$DIR_NAMEtobuild/modules/$DIR_NAME. This modification alters the location where module documentation is generated.Please confirm that this change is intentional and aligns with the project's documentation structure. Run the following script to check for any references to the old path that might need updating:
If this change is intentional, ensure that all related scripts, documentation, and build processes are updated accordingly to use the new path.
✅ Verification successful
Documentation path change verified.
All references to the old
docs/build/modulespath have been removed, and the newbuild/modulespath is consistently used across the scripts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old docs/build/modules path # Test: Search for references to the old path rg --type-add 'script:*.{sh,bash}' --type script 'docs/build/modules' # Test: Check if the new build/modules directory is referenced or used elsewhere rg --type-add 'script:*.{sh,bash}' --type script 'build/modules'Length of output: 1332
julienrbrt
requested review from
facundomedica and
akhilkumarpilli
and removed request for
tac0turtle
|
@julienrbrt do we need this line? We can remove it I guess Line 10 in def2f8e |
(cherry picked from commit fae85e0)
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
MODULE_DIRECTORY.