Feat/add chain messages list #309
Conversation
…s (with fully qualified name) every time the proto definitions are updated
|
Caution Review failedThe pull request is closed. WalkthroughA new Makefile target, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant FileSystem
User->>Makefile: make copy-chain-types
Makefile->>Makefile: Run copy-chain-types steps
Makefile->>Makefile: Invoke extract-message-names
Makefile->>FileSystem: Search for tx.pb.go files in ./chain
Makefile->>FileSystem: Extract proto.RegisterType lines
Makefile->>FileSystem: Parse and filter message names
Makefile->>FileSystem: Write JSON array to injective_data/chain_messages_list.json
Makefile->>User: Print extraction summary
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🧹 Nitpick comments (2)
Makefile (2)
153-154: Use $(MAKE) for recursive Make invocations to inherit flagsReplace the hardcoded call to
make extract-message-nameswith$(MAKE) extract-message-namesso that any flags, parallel jobs (-j), or environment variables passed to the top‐level Make are propagated to the recursive invocation.
161-163: Add explicit check forjqdependencyThe target relies on
jqto format and count messages. Ifjqis missing, the Make target will fail. Consider adding a check at the top of the target:@command -v jq >/dev/null || { echo "Error: jq is required but not installed."; exit 1; }This fails fast with a clear error.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile(2 hunks)injective_data/chain_messages_list.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: run-tests
- GitHub Check: lint
🔇 Additional comments (1)
injective_data/chain_messages_list.json (1)
1-152: Skip review: generated fileThis JSON file is auto-generated by the Makefile’s
extract-message-namestarget. Manual edits are not recommended; it should be updated through the automation.
Makefile
Outdated
| @echo "Extracting message names from tx.pb.go files..." | ||
| @find ./chain -name "tx.pb.go" -exec grep -h "proto\.RegisterType" {} \; | \ | ||
| sed -n 's/.*proto\.RegisterType([^"]*"\([^"]*\)".*/\1/p' | \ | ||
| grep -v "Response$$" | \ |
There was a problem hiding this comment.
Fix grep pattern to correctly exclude Response messages
In the pipeline, grep -v "Response$$" is incorrect because in double quotes $$ expands to the shell PID. To exclude lines ending with Response, use a literal regex anchor, for example:
- grep -v "Response$$" | \
+ grep -v 'Response$' | \This ensures only message types ending with Response are filtered out.
📝 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.
| grep -v "Response$$" | \ | |
| - grep -v "Response$$" | \ | |
| + grep -v 'Response$' | \ |
🤖 Prompt for AI Agents
In the Makefile at line 159, the grep pattern `grep -v "Response$$"` incorrectly
uses double quotes causing `$$` to expand to the shell PID instead of matching
the end of line. To fix this, replace the double quotes with single quotes and
use `grep -v 'Response$'` so that the pattern correctly excludes lines ending
with "Response".
Summary by CodeRabbit
New Features
Chores