-
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: more file reorg #300
fix: more file reorg #300
Conversation
Warning Rate Limit Exceeded@cgilbe27 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 25 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe project underwent a significant reorganization, focusing on TypeScript and GraphQL enhancements, improved error handling, and updated import paths reflecting a new directory structure. Linting rules were refined, test coverage was extended, and utility functions were centralized, indicating a move towards cleaner code architecture and more robust testing practices. 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: 8
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
barrelsby.json
is excluded by:!**/*.json
package.json
is excluded by:!**/*.json
src/sdk/stableswap/mocks/stabletests.csv
is excluded by:!**/*.csv
tsconfig.json
is excluded by:!**/*.json
Files selected for processing (69)
- .eslintrc.js (1 hunks)
- .gitignore (2 hunks)
- codegen.ts (1 hunks)
- jest.config.ts (4 hunks)
- scripts/build.sh (1 hunks)
- scripts/protocgen.sh (1 hunks)
- src/gql/graphql-codegen/codegen-loader.js (1 hunks)
- src/gql/graphql-codegen/codegen-loader.test.ts (1 hunks)
- src/gql/heart-monitor/heart-monitor.test.ts (6 hunks)
- src/gql/heart-monitor/heart-monitor.ts (5 hunks)
- src/gql/index.ts (1 hunks)
- src/gql/query/communityPool.ts (1 hunks)
- src/gql/query/delegations.ts (1 hunks)
- src/gql/query/distributionCommissions.ts (1 hunks)
- src/gql/query/governance.ts (2 hunks)
- src/gql/query/ibc.ts (1 hunks)
- src/gql/query/index.ts (1 hunks)
- src/gql/query/markPriceCandles.ts (1 hunks)
- src/gql/query/oracle.ts (1 hunks)
- src/gql/query/perp.ts (2 hunks)
- src/gql/query/redelegations.ts (1 hunks)
- src/gql/query/spotLpPositions.ts (1 hunks)
- src/gql/query/spotPoolCreated.ts (1 hunks)
- src/gql/query/spotPoolExited.ts (1 hunks)
- src/gql/query/spotPoolJoined.ts (1 hunks)
- src/gql/query/spotPoolSwap.ts (1 hunks)
- src/gql/query/spotPools.ts (1 hunks)
- src/gql/query/stats.ts (2 hunks)
- src/gql/query/unbondings.ts (1 hunks)
- src/gql/query/users.ts (1 hunks)
- src/gql/query/validators.ts (1 hunks)
- src/gql/query/wasm.ts (1 hunks)
- src/gql/subscription/markPriceCandlesSubscription.ts (1 hunks)
- src/gql/subscription/oraclePricesSubscription.ts (1 hunks)
- src/gql/subscription/perpMarketSubscription.ts (1 hunks)
- src/gql/subscription/perpPositionsSubscription.ts (1 hunks)
- src/gql/utils/consts.test.ts (1 hunks)
- src/gql/utils/consts.ts (5 hunks)
- src/gql/utils/defaultObjects.ts (1 hunks)
- src/gql/utils/index.ts (1 hunks)
- src/index.ts (1 hunks)
- src/sdk/balancer/balancer.test.ts (1 hunks)
- src/sdk/index.ts (1 hunks)
- src/sdk/msg/encode-types.ts (1 hunks)
- src/sdk/msg/perp.ts (1 hunks)
- src/sdk/msg/spot.ts (1 hunks)
- src/sdk/query/epochs.test.ts (1 hunks)
- src/sdk/query/inflation.test.ts (1 hunks)
- src/sdk/query/oracle.test.ts (1 hunks)
- src/sdk/query/oracle.ts (1 hunks)
- src/sdk/query/perp.test.ts (1 hunks)
- src/sdk/query/perp.ts (1 hunks)
- src/sdk/query/query.test.ts (3 hunks)
- src/sdk/query/query.ts (1 hunks)
- src/sdk/query/spot.test.ts (2 hunks)
- src/sdk/query/spot.ts (3 hunks)
- src/sdk/query/sudo.test.ts (1 hunks)
- src/sdk/stableswap/stableswap.test.ts (1 hunks)
- src/sdk/tx/signer.test.ts (1 hunks)
- src/sdk/tx/txClient.test.ts (6 hunks)
- src/sdk/utils/chain.test.ts (1 hunks)
- src/sdk/utils/chain.ts (3 hunks)
- src/sdk/utils/faucet.test.ts (1 hunks)
- src/sdk/utils/hash.test.ts (2 hunks)
- src/sdk/utils/hash.ts (1 hunks)
- src/sdk/utils/index.ts (1 hunks)
- src/sdk/utils/parse.test.ts (2 hunks)
- src/sdk/utils/result.ts (2 hunks)
- src/sdk/utils/testutil.ts (2 hunks)
Files skipped from review due to trivial changes (10)
- src/gql/graphql-codegen/codegen-loader.test.ts
- src/gql/index.ts
- src/gql/query/spotPools.ts
- src/gql/utils/index.ts
- src/sdk/msg/perp.ts
- src/sdk/query/inflation.test.ts
- src/sdk/query/perp.test.ts
- src/sdk/query/query.test.ts
- src/sdk/utils/chain.ts
- src/sdk/utils/parse.test.ts
Additional comments: 49
src/sdk/utils/index.ts (1)
- 6-10: The addition of exports for "faucet," "hash," "result," and "testutil" modules is noted. Ensure that these modules are intended to be part of the public API and that their respective tests and documentation are updated to reflect these changes.
Verification successful
The new scripts have successfully found test files for the modules "faucet" and "hash" as indicated by the presence of
faucet.test.ts
andhash.test.ts
in the output. This confirms that there are indeed test cases for these modules. Additionally, references to "faucet," "hash," "result," and "testutil" were found in various files including markdown files, which could be part of the documentation.Given this information, it can be concluded that:
- The "faucet" module has corresponding tests (
faucet.test.ts
).- The "hash" module has corresponding tests (
hash.test.ts
).- The "result" and "testutil" modules are mentioned in documentation files (
result.ts
,testutil.ts
).Therefore, the review comment has been addressed, and the verification process can be concluded with the modules having corresponding tests and documentation references.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the newly exported modules have corresponding tests and documentation fd --type file --exec rg --type ts "describe\(.*faucet.*\)" --files-with-matches fd --type file --exec rg --type ts "describe\(.*hash.*\)" --files-with-matches fd --type file --exec rg --type ts "describe\(.*result.*\)" --files-with-matches fd --type file --exec rg --type ts "describe\(.*testutil.*\)" --files-with-matchesLength of output: 331
Script:
#!/bin/bash # Broaden the search for test files that might contain tests for the newly exported modules fd --type file --exec rg "describe\(" --files-with-matches # Search for documentation files that might contain references to the newly exported modules fd --type file --exec rg --type md "faucet|hash|result|testutil" --files-with-matchesLength of output: 1430
src/sdk/index.ts (1)
- 10-10: The addition of
export * from "./utils/index"
is noted. Ensure that this change does not introduce any circular dependencies and that the exported entities are intended for public use.scripts/build.sh (1)
- 4-19: The build script has been updated to check for the existence of the
./nibiru/
directory and to execute different commands based on that check. Ensure that the script is tested in environments where the./nibiru/
directory both exists and does not exist to verify the correct behavior in each case..eslintrc.js (1)
- 13-13: The addition of
"plugin:@typescript-eslint/recommended"
to the ESLint configuration is noted. Ensure that the codebase is linted with the new rules and that any new linting errors are resolved.src/sdk/msg/encode-types.ts (1)
- 21-21: The change from
export interface TxMessage
toexport type TxMessage = EncodeObject
is noted. Ensure that this change does not affect any consumers of theTxMessage
type and that it is consistent with the intended use of the type.Verification successful
The usage of
TxMessage
in the filessrc/sdk/tx/txClient.test.ts
,src/sdk/msg/spot.ts
, andsrc/sdk/msg/perp.ts
appears to be as a type annotation for arrays and function return types. There is no indication from the current context that the change from an interface to a type alias has affected these usages, assumingEncodeObject
is compatible with the previous interface structure.The reference in
src/sdk/utils/parse.ts
is a comment and does not indicate a code dependency that would be affected by the change.Based on the provided context, there are no issues found with the change from
export interface TxMessage
toexport type TxMessage = EncodeObject
. However, this verification assumes thatEncodeObject
is a suitable replacement for the originalTxMessage
interface. IfEncodeObject
has different properties or methods that are not compatible with the original interface, there could be issues that are not evident from the current context.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that use TxMessage to ensure they are not affected by the change rg --type ts "TxMessage" --files-with-matchesLength of output: 164
Script:
#!/bin/bash # Check the usage of TxMessage in each file to ensure compatibility with the change to a type alias for file in src/sdk/utils/parse.ts src/sdk/tx/txClient.test.ts src/sdk/msg/spot.ts src/sdk/msg/perp.ts src/sdk/msg/encode-types.ts; do echo "File: $file" rg "TxMessage" "$file" doneLength of output: 2327
src/gql/query/index.ts (1)
- 19-19: The reordering of export statements for
spotPoolSwap
andspotPools
is noted. Ensure that this reordering does not affect the import order in dependent modules.Reordering of export statements is typically safe as long as there are no circular dependencies or side effects in the imported modules.
src/gql/graphql-codegen/codegen-loader.js (1)
- 1-1: The addition of the
fetch
import from the "cross-fetch" package is noted. Ensure that thefetch
function is used correctly within the module and that the package is included in the project's dependencies.Verification successful
The
fetch
function from the "cross-fetch" package is present in thepackage.json
dependencies, and the usage offetch
within thesrc/gql/graphql-codegen/codegen-loader.js
file is consistent with standard practices for making a POST request to a GraphQL endpoint.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if "cross-fetch" is included in the package.json dependencies cat package.json | jq '.dependencies["cross-fetch"]'Length of output: 62
Script:
#!/bin/bash # Search for the usage of `fetch` in the codebase rg "fetch\("Length of output: 353
Script:
#!/bin/bash # Examine the usage of `fetch` in the src/gql/graphql-codegen/codegen-loader.js file rg "fetch\(" src/gql/graphql-codegen/codegen-loader.js -C 5Length of output: 398
codegen.ts (1)
- 2-2: The update from
require
to ES6import
forpascalCase
is noted. Ensure that the module is correctly imported and that there are no issues with the module resolution.Switching to ES6 import syntax is a best practice and should not cause issues if the module exports are correctly defined.
scripts/protocgen.sh (1)
- 15-17: The script now checks if the "../nibiru/" directory exists and updates the
NIBIRU_REPO
variable accordingly. This is a good practice to avoid hard-coded paths and to make the script more flexible when the directory structure changes.src/sdk/utils/hash.test.ts (2)
- 46-49: The conditional check for
resBz.ok
and the corresponding assertion are correctly implemented. This ensures that the test only proceeds ifresBz.ok
is truthy, which is a good practice for avoiding null reference errors.- 59-59: The use of optional chaining with
resBz.err?.message
is a good improvement over the non-null assertion operator. It provides better safety against null or undefined values.src/sdk/query/epochs.test.ts (1)
- 2-3: The updated import paths for the
query
module andsetupEpochsExtension
function reflect the reorganization of the project's structure. This change is necessary to maintain the correct references to the modules.src/sdk/query/perp.ts (1)
- 13-13: The import statement for the
fromSdkDec
function has been updated to reflect its new location within the project's directory structure. This change is necessary to maintain the correct reference to the function after the reorganization.src/sdk/utils/hash.ts (1)
- 62-62: The replacement of the non-null assertion operator with the nullish coalescing operator is a good practice. It ensures that an empty array is used when there's no match, which prevents potential runtime errors.
src/sdk/utils/result.ts (2)
- 48-48: The use of
unknown
instead ofany
in theofSafeExec
method improves type safety by not making any assumptions about the type of the arguments or the error.- 75-75: The update to use
unknown
for theerr
parameter in theparseError
function is a good practice for type safety, ensuring that the function can handle any type of error object.src/gql/query/governance.ts (1)
- 1-6: The reorganization of import paths for
defaultGovernance
,convertObjectToPropertiesString
,doGqlQuery
,gqlQuery
, and the generated types is a necessary change to reflect the new structure of the project. This ensures that the correct modules are referenced.src/sdk/tx/signer.test.ts (1)
- 43-48: The addition of new test cases for
newRandomWallet
andnewSignerFromMnemonic
functions is a good practice to ensure the correct functionality and behavior of these functions.src/sdk/msg/spot.ts (1)
- 9-9: The import of
toSdkDec
has been updated to reflect its new location within the project's directory structure. This change is necessary to maintain the correct reference to the function after the reorganization.src/sdk/balancer/balancer.test.ts (1)
- 2-2: The modification of the import statement for
BalancerPool
andSwap
to import from the current directory is a change that simplifies the import path and makes it more readable..gitignore (2)
- 7-11: The addition of
src/gql/utils/generated.ts
,src/gql/utils/schema.graphql
,dist
,**/node_modules/*
, andnibiru/
to the.gitignore
file is a good practice to prevent these directories and files from being tracked by Git, which could lead to unnecessary clutter in the repository.- 14-18: Moving
reports
,junit.xml
,temp
,.build-cache
, and.DS_STORE
to the end of the ignore list is a minor change that does not affect the functionality of the.gitignore
file but may improve readability.src/sdk/utils/testutil.ts (2)
- 2-2: The updated import paths for
Chain
andLocalnet
reflect the reorganization of the project's structure. This change is necessary to maintain the correct references to the modules.- 26-28: The change in the parameter type in the
assertValidBlockFromJsonRpc
function from a specific type to a dictionary with unknown values is a good practice for making the function more generic and adaptable to different block structures.src/sdk/utils/faucet.test.ts (1)
- 4-6: The reorganization of imports and updating of import paths for related modules in
faucet.test.ts
is a necessary change to reflect the new structure of the project. This ensures that the correct modules are referenced.src/gql/utils/consts.ts (2)
- 3-11: The addition of the new type
IterableDictionary<T>
and its usage in various functions is a good practice for creating more generic and reusable code. This change improves the logic and functionality of the code by allowing for iteration over dictionary types.- 13-16: The function
doGqlQuery
now returns a generic type, which is a good practice for making the function more adaptable to different return types and improving type safety.src/gql/query/perp.ts (1)
- 6-11: The reorganization of the import paths for default objects and utility functions in
perp.ts
is a necessary change to reflect the new structure of the project. This ensures that the correct modules are referenced.src/sdk/query/query.ts (1)
- 29-30: The import paths for
Result
andbytesToHex
,hexToBytes
have been updated to reflect their new locations within theutils
directory. This change is consistent with the PR's objective of reorganizing the codebase.src/gql/query/stats.ts (2)
- 9-14: The import paths for
defaultObjects
andconsts
have been updated to reflect their new locations under theutils
directory. This change aligns with the PR's objective of improving the project structure.- 31-31: The import path for
generated
types has been updated to reflect the new location under theutils
directory. This change is consistent with the PR's objective of reorganizing the codebase.src/sdk/utils/chain.test.ts (1)
- 14-21: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [17-22]
The import paths for modules within the same directory have been updated to use relative paths. This change is a good practice for internal module imports and is consistent with the PR's objective of improving the project structure.
src/sdk/query/oracle.ts (1)
- 34-34: The import statement for
fromSdkDec
has been updated to reflect its new location within theutils
module. This change is consistent with the PR's objective of reorganizing the codebase.src/sdk/tx/txClient.test.ts (4)
- 8-10: The import paths for
Msg*
types andDirection
enum have been updated to reflect their new locations within theprotojs
directory. This change is consistent with the PR's objective of reorganizing the codebase.- 22-22: The import path for
testutil
has been updated to reflect its new location within theutils
directory. This change is consistent with the PR's objective of reorganizing the codebase.- 157-162: The error handling logic has been refined to better capture and process error logs. This change aims to improve error management within the test file.
- 224-229: The error handling logic has been refined to better capture and process error logs. This change aims to improve error management within the test file.
src/sdk/query/spot.ts (4)
- 39-39: The import statement for
fromSdkDec
has been updated to reflect its new location within theutils
module. This change is consistent with the PR's objective of reorganizing the codebase.- 49-49: The
transformPool
function now requires a non-nullablePool
parameter instead of an optional one. This change improves type safety by ensuring that the function always receives aPool
object.- 116-116: The conditional application of the
transformPool
function to thepool
property of theQueryPoolResponse
object is a good practice, as it ensures that the transformation is only applied if thepool
property is present.- 122-122: The
transformPool
function is now applied to eachpool
in thepools
array of theQueryPoolsResponse
object. This ensures that all pools are consistently transformed according to the new structure.src/sdk/query/oracle.test.ts (1)
- 1-8: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [90-95]
The
transformPool
function has been modified to accept an object with specific properties. Ensure that all calls totransformPool
throughout the codebase have been updated to pass the correct arguments.src/gql/heart-monitor/heart-monitor.ts (2)
- 86-92: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [39-100]
Imports have been reorganized, and certain types and functions have been moved to different directories. Verify that the new import paths are correct and that the modules are exported from their respective new locations.
Verification successful
The verification process has confirmed that the import paths are correct and the modules are exported from their respective new locations as expected. The output from the scripts lists the export statements from the 'utils', 'query', 'subscription', and 'queryBatchHandler' modules, which align with the import statements in the provided code snippet.
* 172-174: The `GQLQueryGqlBatchHandler` function is now a generic function. Ensure that the generic type `T` is being used correctly within the function and that all calls to this function are updated accordingly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the updated import paths are correct and the modules exist at the new locations. fd 'utils' --exec rg --type typescript 'export .* utils' fd 'query' --exec rg --type typescript 'export .* query' fd 'subscription' --exec rg --type typescript 'export .* subscription' fd 'queryBatchHandler' --exec rg --type typescript 'export .* queryBatchHandler' fd 'wasm' --exec rg --type typescript 'export .* wasm'Length of output: 810
Script:
#!/bin/bash # Verify that the updated import paths are correct and the modules exist at the new locations. # Correcting the file type for TypeScript in ripgrep to `ts` fd 'utils' --exec rg --type ts 'export .* utils' fd 'query' --exec rg --type ts 'export .* query' fd 'subscription' --exec rg --type ts 'export .* subscription' fd 'queryBatchHandler' --exec rg --type ts 'export .* queryBatchHandler' fd 'wasm' --exec rg --type ts 'export .* wasm'Length of output: 341
Script:
#!/bin/bash # Broaden the search pattern to capture any export statements in the relevant files. fd 'utils' --exec rg --type ts 'export' fd 'query' --exec rg --type ts 'export' fd 'subscription' --exec rg --type ts 'export' fd 'queryBatchHandler' --exec rg --type ts 'export .* queryBatchHandler' # For 'wasm', we already have output, so we do not need to run this search again.Length of output: 16942
src/sdk/query/spot.test.ts (2)
- 2-4: The import paths for the
query
module have been updated. Ensure that the new paths are correct and that the module is exported from the new location.- 90-95: The
transformPool
function has been modified to accept an object with specific properties instead of no arguments. Ensure that all calls totransformPool
throughout the codebase have been updated to pass the correct arguments.src/gql/heart-monitor/heart-monitor.test.ts (3)
- 3-3: The import statement for
checkFields
has been updated to reflect its new location in theutils
module. Ensure that theutils
module is the correct new location for this function.Verification successful
The import statement for
checkFields
from the../utils
module in theheart-monitor.test.ts
file has been verified and is correct as per the provided output.* 226-226: The default value of `ibcChannels` has been changed to `undefined`. This change simplifies the test by not requiring a default response to be passed when it's not needed. Ensure that this change does not affect other tests that might rely on the previous default value.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that checkFields is present in the utils module. ast-grep --lang typescript --pattern $'import { $$$, checkFields, $$$ } from "../utils"'Length of output: 210
Verification successful
The search for
ibcChannels: defaultIbcChannelsResponse
within TypeScript files has yielded two occurrences, both within the same filesrc/gql/heart-monitor/heart-monitor.test.ts
. This indicates that thedefaultIbcChannelsResponse
is indeed used in the tests for the heart monitor GraphQL functionality. Since no other files are affected, it seems that the change toundefined
foribcChannels
in the test defaults only impacts theheart-monitor.test.ts
file.* 19-21: The import paths for several modules have been updated. Confirm that these new paths are correct and that the modules are located as expected in the new project structure.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that no other tests are affected by the change to ibcChannels default value. rg --type typescript "ibcChannels: defaultIbcChannelsResponse"Length of output: 99
Script:
#!/bin/bash # Verify that no other tests are affected by the change to ibcChannels default value. # This time, we specify TypeScript files manually using the -g flag. rg "ibcChannels: defaultIbcChannelsResponse" -g '*.ts' -g '*.tsx'Length of output: 249
Verification successful
The verification script has successfully found instances of the import statement
from "../query"
in the codebase. This confirms that the updated import paths are indeed being used in the codebase, which aligns with the changes mentioned in the review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the updated import paths are correct. fd --type file --exec rg --type typescript "from \"../query\""Length of output: 3632
Script:
#!/bin/bash # Verify that the updated import paths are correct by searching for the import statement. fd --type file --exec rg "from \"../query\""Length of output: 217
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 ignored due to path filters (1)
package.json
is excluded by:!**/*.json
Files selected for processing (1)
- jest.config.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- jest.config.ts
## [3.0.0](v2.0.1...v3.0.0) (2024-02-05) ### Bug Fixes * fix merge ([4f93be8](4f93be8)) * gql where args ([#298](#298)) ([b57ef0a](b57ef0a)) * more file reorg ([#300](#300)) ([d6a053a](d6a053a)) ### Miscellaneous Chores * pr title lint workflow ([#293](#293)) ([b5dd050](b5dd050)) ### Code Refactors * tests ([#301](#301)) ([e14b83b](e14b83b)) [skip ci]
🎉 This PR is included in version 3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
.gitignore
to properly exclude generated files and directories.Documentation
Chores