-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix: chore msg factories #323
Conversation
WalkthroughThe updates encompass enhancements and the introduction of new functionalities across various modules, primarily focusing on message encoding and decoding for specific operations like 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 (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (10)
- src/gql/query/index.ts (1 hunks)
- src/sdk/msg/devgas.ts (1 hunks)
- src/sdk/msg/encode-types.ts (1 hunks)
- src/sdk/msg/index.ts (1 hunks)
- src/sdk/msg/inflation.ts (1 hunks)
- src/sdk/msg/oracle.ts (1 hunks)
- src/sdk/msg/sudo.ts (1 hunks)
- src/sdk/msg/tokenfactory.ts (1 hunks)
- src/sdk/query/epochs.test.ts (1 hunks)
- src/sdk/query/epochs.ts (2 hunks)
Additional comments: 4
src/sdk/msg/index.ts (1)
- 5-12: The addition of exports for
devgas
,inflation
,oracle
,sudo
, andtokenfactory
modules aligns with the PR's objectives and follows TypeScript best practices for module exports. These changes enhance the project's messaging capabilities as intended.src/gql/query/index.ts (1)
- 13-22: The reordering of exports in
src/gql/query/index.ts
appears to be aimed at improving organization. While generally safe, it's recommended to verify that this change does not impact dependent modules, especially if there are any load order dependencies or implicit assumptions about the order of exports.Verification successful
The verification process did not find direct imports of the reordered modules
markPriceCandles
andspotPoolSwap
using the specific search pattern provided. This suggests that the reordering of exports insrc/gql/query/index.ts
is unlikely to impact dependent modules, assuming that imports are correctly specified elsewhere in the project. It's generally safe to reorder exports in TypeScript/JavaScript projects without affecting functionality. However, it's always good practice to perform thorough testing to ensure no unintended side effects occur.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if the reordering affects any dependent modules explicitly relying on the previous order. # This script is a placeholder and should be adjusted based on the project's specific verification needs. echo "Manual verification recommended to ensure no impact on dependent modules."Length of output: 156
Script:
#!/bin/bash # Search for imports of the reordered modules across the project to verify if the reordering has any impact. # List of reordered modules for reference modules=("markPriceCandles" "spotPoolSwap") # Loop through each module and search for imports across the project for module in "${modules[@]}"; do echo "Searching for imports of $module..." rg --glob '!node_modules/*' "from './$module'" echo "----------------------------------------" doneLength of output: 601
src/sdk/msg/encode-types.ts (1)
- 2-23: The updates to
MsgFactory
inencode-types.ts
, including the addition of new message factories forDevgas
,Inflation
,Oracle
,Sudo
, andTokenfactory
, align with the PR's objectives to enhance the project's messaging capabilities. Ensure that these new factories are correctly implemented and integrated within the project.Verification successful
The verification process confirms the addition and correct implementation of new message factories (
DevgasMsgFactory
,InflationMsgFactory
,OracleMsgFactory
,SudoMsgFactory
,TokenfactoryMsgFactory
) inencode-types.ts
, as well as their correct import and usage within the project. This aligns with the objectives mentioned in the review comment, enhancing the project's messaging capabilities.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Placeholder script for verifying the correct implementation and integration of new message factories. echo "Manual verification recommended to ensure correct implementation and integration of new message factories."Length of output: 222
Script:
#!/bin/bash # Searching for class definitions of the new message factories to confirm their existence. ast-grep --lang typescript --pattern $'class $_ extends $_ { $$$ }' | grep -E 'DevgasMsgFactory|InflationMsgFactory|OracleMsgFactory|SudoMsgFactory|TokenfactoryMsgFactory' # Checking for import statements of these factories in `encode-types.ts` and possibly other references in the project. rg 'DevgasMsgFactory|InflationMsgFactory|OracleMsgFactory|SudoMsgFactory|TokenfactoryMsgFactory' -ttsLength of output: 1133
src/sdk/query/epochs.ts (1)
- 6-15: The renaming of
QueryEpochsInfoRequest
toQueryEpochInfosRequest
andQueryEpochsInfoResponse
toQueryEpochInfosResponse
inepochs.ts
improves naming clarity and consistency. Ensure that all references to these interfaces throughout the project have been updated to reflect the new names.Verification successful
The renaming of
QueryEpochsInfoRequest
toQueryEpochInfosRequest
andQueryEpochsInfoResponse
toQueryEpochInfosResponse
inepochs.ts
has been successfully verified. No occurrences of the old interface names were found in the codebase, indicating that all references have been updated to reflect the new names.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Placeholder script for verifying that all references to the renamed interfaces have been updated. echo "Manual verification recommended to ensure all references to the renamed interfaces have been updated."Length of output: 212
Script:
#!/bin/bash # Searching for occurrences of the old interface names to verify if they've been updated. rg "QueryEpochsInfoRequest|QueryEpochsInfoResponse"Length of output: 53
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/test-ts-sdk.yaml (2 hunks)
Additional comments: 2
.github/workflows/test-ts-sdk.yaml (2)
- 32-32: The update of the reference branch from
releases/v0.21.x
tomain
for thenibiru
repository checkout step is noted. Ensure that themain
branch contains all necessary changes and is stable for use in this workflow. This change could affect the behavior of subsequent steps if themain
branch is significantly different from the previously used branch. It's important to verify that all dependencies and configurations in themain
branch are compatible with thets-sdk
project to avoid build or test failures.- 29-35: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [28-32]
Although the removal of a checkout step is mentioned in the summary, the provided code snippet still includes the checkout step for the
nibiru
repository. If the removal has indeed taken place elsewhere in the workflow, it's crucial to assess the impact of this change on the workflow's functionality. The checkout of thenibiru
repository might be necessary for certain build or test steps, especially if they depend on artifacts or configurations from that repository. Verify that all necessary components are still accessible and that the workflow can function correctly without this step.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- scripts/build.sh (1 hunks)
- scripts/protocgen.sh (1 hunks)
Additional comments: 2
scripts/build.sh (1)
- 8-8: Switching the git branch from
releases/v0.21.x
tomain
in thebuild.sh
script aligns the build process with the latest version of the codebase. Ensure to verify the impact of this change on any dependencies or specific version requirements that might have relied on thereleases/v0.21.x
branch.Also applies to: 12-12
scripts/protocgen.sh (1)
- 49-49: The updates to the
protocgen.sh
script, including the removal of specific directories and adjustments to exports, aim to streamline the project and manage dependencies more effectively. It's important to thoroughly test the generated TypeScript code to ensure that these changes have not inadvertently removed necessary functionality or broken existing dependencies.Also applies to: 53-54, 57-58, 61-61
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.
Failure is sonarqube |
LGTM |
🎉 This PR is included in version 3.0.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #320
Adds all missing msg factories
Summary by CodeRabbit
New Features
devgas
,inflation
,oracle
,sudo
, andtokenfactory
modules.Refactor
encode-types.ts
to support new modules.Tests
epochs.test.ts
to align with updated request and response naming conventions.Documentation