fix(proto): remove conditional preventing proper generated file placement #20650
Conversation
WalkthroughWalkthroughThe recent changes focus on refining the error handling and file management in two bash scripts: Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@kocubinski your pull request is missing a changelog! |
kocubinski
changed the title
kocubinski
changed the title
![coderabbitai[bot]](https://github.com/avatars/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 (3)
scripts/go-lint-all.bash (1)
Line range hint
48-48: Double quote array expansions to avoid re-splitting elements.- for dir in ${GIT_DIFF[@]}; do + for dir in "${GIT_DIFF[@]}"; doThis change ensures that each element in the array is treated as a single word, even if it contains spaces. This is crucial in scripts where directory names or file paths might contain spaces.
scripts/protocgen.sh (2)
Line range hint
24-24: Replace-awith&&in conditions for better portability and readability.- if [ -f "buf.gen.pulsar.yaml" -a -d "../cosmos" -a "$dir" != "./proto" ]; then + if [ -f "buf.gen.pulsar.yaml" ] && [ -d "../cosmos" ] && [ "$dir" != "./proto" ]; then - if [ -d "../cosmossdk.io" -a "$dir" != "./proto" ]; then + if [ -d "../cosmossdk.io" ] && [ "$dir" != "./proto" ]; thenThis change enhances the script's compatibility with different shells and improves the clarity of the conditions.
Also applies to: 47-47
Line range hint
32-32: Usefind -execor awhile readloop instead of aforloop overfindoutput.- for file in $(find . -maxdepth 5 -name '*.proto'); do + find . -maxdepth 5 -name '*.proto' -exec bash -c 'buf generate --template buf.gen.gogo.yaml "$1"' _ {} \;This change prevents issues related to word splitting and globbing, making the script more robust, especially when handling filenames with spaces or unusual characters.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
x/consensus/types/consensus.pb.gois excluded by!**/*.pb.go
Files selected for processing (2)
- scripts/go-lint-all.bash (1 hunks)
- scripts/protocgen.sh (1 hunks)
Additional context used
Shellcheck
scripts/go-lint-all.bash
[error] 48-48: Double quote array expansions to avoid re-splitting elements. (SC2068)
scripts/protocgen.sh
[warning] 24-24: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. (SC2166)
[warning] 32-32: For loops over find output are fragile. Use find -exec or a while read loop. (SC2044)
[warning] 47-47: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. (SC2166)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
scripts/protocgen.sh (3)
Line range hint
24-24: Refactor the conditional check to use&&for clarity and to adhere to best practices.- if [ -d "../cosmos" -a "$dir" != "./proto" ]; then + if [ -d "../cosmos" ] && [ "$dir" != "./proto" ]; then
Line range hint
32-32: Consider replacing the for loop with afind -execor awhile readloop to handle filenames with spaces or unusual characters more robustly.- for file in $(find . -maxdepth 5 -name '*.proto'); do + find . -maxdepth 5 -name '*.proto' -exec sh -c 'buf generate --template buf.gen.gogo.yaml "$1"' _ {} \;
Line range hint
47-47: Refactor the conditional check to use&&for clarity and to adhere to best practices.- if [ -d "../github.com" -a "$dir" != "./proto" ]; then + if [ -d "../github.com" ] && [ "$dir" != "./proto" ]; then
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- scripts/protocgen.sh (1 hunks)
Additional context used
Shellcheck
scripts/protocgen.sh
[warning] 24-24: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. (SC2166)
[warning] 32-32: For loops over find output are fragile. Use find -exec or a while read loop. (SC2044)
[warning] 47-47: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. (SC2166)
Additional comments not posted (1)
scripts/protocgen.sh (1)
42-42: Ensure the directory check and operations are correctly formatted for better readability and maintenance.- if [ -d "../cosmossdk.io" ]; then + if [ -d "../cosmossdk.io" ]; thenLikely invalid or redundant comment.
|
utACK |
* main: fix(x/staking): stop validators from rotating to the same key on the same block (#20649) perf: add cache to address codec (#20122) build(deps): Bump google.golang.org/protobuf from 1.34.1 to 1.34.2 (#20632) fix: remove recipient amount from map (#20625) fix(proto): remove conditional preventing proper generated file placement (#20650) (serverv2/cometbft) Read config from commands & handle `FlagNode` (#20621) fix(x/consensus): fix .proto file placement (#20646) fix(store): avoid nil error on not exhausted payload stream (#20644) fix (x/accounts): Fix genesis condition check (#20645) feat(accounts): add genesis account initialization (#20642) fix(x/gov): limit execution in gov (#20348)
Description
Follow up to #20646
I don't know why the conditional was there, but it was preventing files from getting moved to the right place.
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
go-lint-allscript for better reliability.protocgenscript for efficiency.