-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update/core docs #67
base: main
Are you sure you want to change the base?
Update/core docs #67
Conversation
WalkthroughThis pull request involves comprehensive configuration updates across multiple packages in the Garden SDK. The changes primarily focus on improving module resolution, build configurations, and TypeScript compilation settings. Key modifications include updating module resolution strategies to "bundler", transitioning from CommonJS to ES2020 module formats, refining Vite configurations, and enhancing build output settings. The changes aim to standardize project configurations, improve build processes, and ensure better compatibility with modern JavaScript and TypeScript ecosystems. Changes
Poem
Finishing Touches
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
CodeRabbit Configuration 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.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/core/vite.config.ts (1)
28-31
:⚠️ Potential issueReevaluate including
devDependencies
inexternal
configurationAs previously mentioned, including
devDependencies
in theexternal
array might exclude necessary modules from the bundle if they are used at runtime. It's generally advisable to include onlydependencies
and peer dependencies.Consider adjusting the
external
configuration:external: [ // Explicitly handle all dependencies as external ...Object.keys(pkg.dependencies || {}), - ...Object.keys(pkg.devDependencies || {}), /node_modules/, // This pattern will match all node_modules ],
packages/orderbook/vite.config.ts (1)
29-31
:⚠️ Potential issueReevaluate including
devDependencies
inexternal
configurationIncluding
devDependencies
may inadvertently exclude necessary modules from your build if they are used at runtime. It's typically safer to include onlydependencies
.Consider modifying the
external
array:external: [ // Explicitly handle all dependencies as external ...Object.keys(pkg.dependencies || {}), - ...Object.keys(pkg.devDependencies || {}), /node_modules/, // This pattern will match all node_modules ],
🧹 Nitpick comments (9)
packages/react-hooks/vite.config.ts (1)
21-21
: Changingentry
to an array allows for multiple entry pointsConverting
entry
to an array format enables the configuration to support multiple entry points. Since only'src/index.ts'
is specified currently, consider whether this anticipates future additions or if using a string suffices for now.packages/core/vite.config.ts (2)
21-21
: Changingentry
to an array allows for scalabilityUsing an array for
entry
enables support for multiple entry points in the future. If only one entry point is needed, you may opt to keep it as a string for simplicity.
28-52
: Consider extracting common Rollup configurations into a shared moduleSince multiple packages share similar Rollup configurations, abstracting common settings into a shared module can improve maintainability and reduce duplication.
packages/orderbook/vite.config.ts (2)
21-21
: Changingentry
to an array for potential scalabilityWhile the array format for
entry
allows for future scalability with multiple entry points, if only a single entry is planned, consider whether this change is necessary.
28-52
: Extract common configurations to improve maintainabilitySince similar configurations are used across multiple packages, consider abstracting them into a shared configuration file to reduce redundancy.
packages/utils/vite.config.ts (1)
38-44
: Consider simplifying the entryFileNames logicThe current conditional logic for entryFileNames could be simplified since we're already excluding node_modules through the external configuration.
- entryFileNames: ({ name: fileName }) => { - // Ensure we're only processing source files - if (fileName.includes('node_modules')) { - return '[name].js'; - } - return `${fileName}.js`; - }, + entryFileNames: '[name].js',packages/walletConnectors/vite.config.ts (1)
Line range hint
1-55
: Consider configuration sharingThe configuration is nearly identical to utils package. Consider extracting common configuration into a shared base config.
Create a shared base configuration in the root directory that can be extended by each package.
readme.md (1)
13-15
: Consider varying the bullet point descriptions.While the content is accurate, the repetitive sentence structure (all starting with "-") could be improved for better readability.
Consider this variation:
-- [@gardenfi/utils](./packages/utils/README.md): Utilities for the SDK. -- [@gardenfi/react-hooks](./packages/react-hooks/README.md): React hooks for the SDK. -- [@gardenfi/wallet-connectors](./packages/walletConnectors/README.md): Wallet connectors for the SDK. +• [@gardenfi/utils](./packages/utils/README.md): Common utility functions and helpers for the SDK. +• [@gardenfi/react-hooks](./packages/react-hooks/README.md): Collection of React hooks for seamless SDK integration. +• [@gardenfi/wallet-connectors](./packages/walletConnectors/README.md): Comprehensive wallet connection handlers for the SDK.🧰 Tools
🪛 LanguageTool
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... orders once you setup your wallets. - @gardenfi/utils: Uti...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s/README.md): Utilities for the SDK. - [@gardenfi/react-hooks](./packages/react-hooks/REA...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...README.md): React hooks for the SDK. - [@gardenfi/wallet-connectors](./packages/walletCon...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
packages/core/README.md (1)
173-177
: Consider removing the trailing colon in the heading.The "Top contributors:" heading contains trailing punctuation.
-### Top contributors: +### Top Contributors🧰 Tools
🪛 Markdownlint (0.37.0)
173-173: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.gitignore
(1 hunks)packages/core/README.md
(3 hunks)packages/core/package.json
(1 hunks)packages/core/tsconfig.json
(1 hunks)packages/core/tsconfig.spec.json
(1 hunks)packages/core/vite.config.ts
(1 hunks)packages/orderbook/tsconfig.json
(1 hunks)packages/orderbook/tsconfig.spec.json
(1 hunks)packages/orderbook/vite.config.ts
(1 hunks)packages/react-hooks/tsconfig.json
(2 hunks)packages/react-hooks/tsconfig.spec.json
(1 hunks)packages/react-hooks/vite.config.ts
(1 hunks)packages/utils/tsconfig.json
(1 hunks)packages/utils/tsconfig.spec.json
(1 hunks)packages/utils/vite.config.ts
(1 hunks)packages/walletConnectors/tsconfig.json
(1 hunks)packages/walletConnectors/tsconfig.lib.json
(1 hunks)packages/walletConnectors/tsconfig.spec.json
(1 hunks)packages/walletConnectors/vite.config.ts
(1 hunks)readme.md
(2 hunks)tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 LanguageTool
readme.md
[style] ~1-~1: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 1820 characters long)
Context: [![NPM Version](https://img.shields.io/npm...
(EN_EXCESSIVE_EXCLAMATION)
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... orders once you setup your wallets. - @gardenfi/utils: Uti...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s/README.md): Utilities for the SDK. - [@gardenfi/react-hooks](./packages/react-hooks/REA...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...README.md): React hooks for the SDK. - [@gardenfi/wallet-connectors](./packages/walletCon...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Markdownlint (0.37.0)
packages/core/README.md
21-21: null
Link fragments should be valid
(MD051, link-fragments)
22-22: null
Link fragments should be valid
(MD051, link-fragments)
23-23: null
Link fragments should be valid
(MD051, link-fragments)
173-173: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (34)
packages/react-hooks/vite.config.ts (5)
13-15
: Updates todts
plugin configuration enhance TypeScript declaration generationThe changes to
entryRoot
,include
, andexclude
in thedts
plugin configuration ensure that TypeScript declarations are generated accurately from the source files while excluding unnecessary files likenode_modules
.
19-19
: Disabling minification withminify: false
Setting
minify: false
will prevent code minification, resulting in larger build outputs. Please verify that this is intentional and aligns with your distribution and performance strategies.
24-24
: DynamicfileName
improves clarity of output filesUsing a function for
fileName
to include the format in the output filenames enhances clarity and helps distinguish between different module formats.
38-44
: Verify logic inentryFileNames
functionThe
entryFileNames
function customizes output filenames and skips files fromnode_modules
. Since yourentryRoot
is'./src'
, files fromnode_modules
should not be part of the entry files. Please verify if this condition is necessary and that the logic functions as intended.
55-55
: Enabling source maps withsourcemap: true
enhances debuggingGenerating source maps is beneficial for debugging and development. This addition is a positive improvement.
packages/core/vite.config.ts (5)
13-15
: Updates todts
plugin configuration enhance TypeScript declaration generationAdjusting
entryRoot
,include
, andexclude
ensures accurate generation of TypeScript declarations by focusing on the source files and excluding unnecessary directories.
19-19
: Disabling minification withminify: false
Disabling minification will result in larger bundle sizes, which may affect load times in production. Verify that this aligns with your performance and deployment strategies.
24-24
: DynamicfileName
enhances output file namingEmploying a function for
fileName
to include the format in filenames improves the clarity and organization of output files.
38-44
: Confirm logic inentryFileNames
functionEnsure that the custom logic in
entryFileNames
correctly handles all cases and that the condition checking for'node_modules'
is necessary. This will prevent potential issues with output file naming.
55-55
: Enabling source maps enhances debugging capabilitiesAdding
sourcemap: true
is beneficial for debugging and aligns with best practices for development.packages/orderbook/vite.config.ts (5)
13-15
: Updates todts
plugin configuration enhance TypeScript declaration generationThe adjustments to
entryRoot
,include
, andexclude
sharpen focus on the relevant source files for type declaration generation, improving the build process.
19-19
: Disabling minification withminify: false
Turning off minification increases bundle size, which could impact application performance. Verify that this decision aligns with your deployment objectives.
24-24
: DynamicfileName
based on format improves build output clarityCustomizing
fileName
to include the format enhances the readability of your output files and aligns with standard practices.
38-44
: Verify the logic withinentryFileNames
functionDouble-check that the condition in
entryFileNames
appropriately handles the file naming, especially regarding entries potentially containing'node_modules'
.
55-55
: Addingsourcemap: true
enhances debuggingEnabling source maps is a good practice for debugging and development purposes.
packages/utils/vite.config.ts (5)
5-5
: LGTM: ESLint integration addedGood addition of the ESLint plugin for static analysis during build.
Also applies to: 9-9
13-15
: LGTM: Improved DTS configurationGood improvements to type declaration generation:
- Explicit source directory specification
- Clear include/exclude patterns
27-32
: LGTM: Comprehensive external dependencies handlingGood approach to handling external dependencies:
- Includes both dependencies and devDependencies
- Catches all node_modules via regex
48-52
: LGTM: Modern output configurationGood use of modern JavaScript features in the output configuration.
21-21
: Verify entry point pathThe entry point is set to
src/index.ts
. Please ensure this file exists and contains the intended exports.Also applies to: 24-24
✅ Verification successful
Entry point path is correct ✅
The entry point
src/index.ts
exists and follows the standard package structure, with proper organization of modules under thelib/
directory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify entry point existence and exports test -f src/index.ts && echo "Entry point exists" || echo "Missing entry point"Length of output: 71
Script:
#!/bin/bash # Check if the entry point exists in the correct location test -f packages/utils/src/index.ts && echo "Entry point exists" || echo "Missing entry point" # List TypeScript files in the src directory echo -e "\nTypeScript files in packages/utils/src:" fd -e ts . packages/utils/srcLength of output: 1044
packages/walletConnectors/vite.config.ts (1)
14-14
: LGTM: Package-specific include patternAppropriate inclusion of
.tsx
files for this React-related package.packages/react-hooks/tsconfig.spec.json (1)
5-5
: Verify test runner compatibilityThe change from CommonJS to ES2020 modules might affect test execution. Please verify that your test runner supports ES modules.
✅ Verification successful
ES2020 module configuration is compatible with test setup
The change to ES2020 modules in tsconfig.spec.json is safe as:
- Package already uses ES modules by default
- Vitest 1.6.0 fully supports ES modules and TypeScript
- No CommonJS require() statements found in test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json for test runner configuration grep -r "test" package.jsonLength of output: 184
Script:
#!/bin/bash # Check for Vitest config files and test files using require echo "=== Checking for Vitest config files ===" fd vitest.config echo -e "\n=== Checking for require() usage in test files ===" rg "require\(" -t ts -t tsx packages/react-hooksLength of output: 305
Script:
#!/bin/bash # Search with corrected file type handling echo "=== Checking for require() in test files ===" rg "require\(" packages/react-hooks/**/*.{ts,tsx,test.ts,test.tsx,spec.ts,spec.tsx} echo -e "\n=== Checking for test setup files ===" fd "setup|test" packages/react-hooks echo -e "\n=== Checking package.json for test config ===" cat packages/react-hooks/package.jsonLength of output: 2039
packages/orderbook/tsconfig.spec.json (1)
5-5
: LGTM: Consistent module formatGood consistency with other packages in using ES2020 modules.
packages/utils/tsconfig.spec.json (1)
5-5
: Verify test runner compatibility with ES2020 modulesThe change from CommonJS to ES2020 modules needs verification with the test runner configuration to ensure tests execute correctly.
Run this script to check the test runner configuration:
✅ Verification successful
ES2020 module format is compatible with the test setup
The codebase uses Vitest with Vite, which natively supports ES modules. All test files already use ES module imports, confirming full compatibility with the ES2020 module format.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for test runner configuration files and their module settings echo "Checking test runner configurations..." fd -e json -e js -e cjs "jest.config|vitest.config" | xargs catLength of output: 152
Script:
#!/bin/bash # Check package.json files for test configurations and dependencies echo "Checking package.json files..." fd package.json | xargs cat | jq 'select(.scripts != null) | {name: .name, scripts: .scripts, dependencies: .dependencies, devDependencies: .devDependencies}' # Look for test setup files echo -e "\nChecking for test setup files..." fd -e ts -e js "setup|test.config" # Search for test runner imports/requires in test files echo -e "\nChecking test file imports..." rg -g '*.spec.ts' -g '*.test.ts' "from ['\"](jest|vitest|mocha|ava)" -A 1Length of output: 8423
packages/core/tsconfig.spec.json (1)
5-5
: LGTM! Maintains consistency with other packagesThe module configuration change aligns with the project-wide transition to ES2020 modules.
packages/walletConnectors/tsconfig.lib.json (1)
10-10
: LGTM! More maintainable include patternsThe switch to pattern-based includes is more maintainable and future-proof. However, let's verify the included files match expectations.
Run this script to check included files:
✅ Verification successful
✅ Verified: Include patterns working as intended
All matched files are legitimate source files within expected locations. The patterns correctly include necessary TypeScript and React components while excluding test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # List all files that would be included by the new patterns echo "Files matching the new include patterns:" cd packages/walletConnectors && \ fd -e ts -e json -e tsx -t f "." src/ --exclude "*.spec.*" --exclude "*.test.*"Length of output: 806
tsconfig.json (1)
8-8
: Verify build tool compatibility with new module settingsThe changes to
module
andmoduleResolution
options optimize for modern bundlers, but we should verify compatibility with the project's build tools.Run this script to check build configurations:
Also applies to: 12-12
✅ Verification successful
Build configuration is compatible with ESNext modules
The project's build setup fully supports ESNext modules and bundler resolution:
- Vite configurations output both ES and CommonJS formats
- All packages use modern module settings consistently
- TypeScript configurations are properly aligned across packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for build tool configurations echo "Checking build tool configurations..." fd -e js -e cjs -e mjs "vite.config|webpack.config|rollup.config" | xargs cat # Check package.json build scripts echo -e "\nChecking build scripts..." fd "package.json" | xargs jq '.scripts | with_entries(select(.key | contains("build")))'Length of output: 607
Script:
#!/bin/bash # Check for Vite config files with .ts extension echo "Checking Vite TypeScript configurations..." fd "vite.config.ts" | xargs cat # Also check for any TypeScript configurations in packages echo -e "\nChecking package TypeScript configurations..." fd "tsconfig" | grep -v "node_modules" | xargs catLength of output: 15721
packages/utils/tsconfig.json (1)
13-14
: Verify TypeScript version compatibility for bundler moduleResolutionThe
"moduleResolution": "bundler"
option requires TypeScript 5.0 or later. Please ensure the project's TypeScript version is compatible.✅ Verification successful
TypeScript version is compatible with bundler moduleResolution
All packages use TypeScript 5.2.2 or higher, which fully supports the
"moduleResolution": "bundler"
option introduced in TypeScript 5.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check TypeScript version in package.json files echo "Checking TypeScript version in package.json files:" find . -name "package.json" -exec sh -c 'echo "{}:" && cat {} | jq -r ".dependencies.typescript, .devDependencies.typescript"' \;Length of output: 8953
packages/walletConnectors/tsconfig.json (1)
14-15
: Configuration aligns with other packagesThe moduleResolution change is consistent with the project-wide update.
packages/core/tsconfig.json (1)
9-9
: Verify build compatibility with dependent packagesAs this is the core package, please ensure all dependent packages are compatible with the bundler moduleResolution strategy.
✅ Verification successful
moduleResolution setting is consistent across packages
All packages in the monorepo are already using the "bundler" moduleResolution strategy, making this change consistent with the project-wide TypeScript configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all package.json files that depend on @garden/core echo "Checking dependencies on @garden/core:" find . -name "package.json" -exec sh -c 'if grep -q "@garden/core" "{}"; then echo "Found in: {}"; fi' \;Length of output: 193
Script:
#!/bin/bash # Check the package name in core's package.json echo "Core package.json contents:" cat packages/core/package.json echo -e "\nFinding all tsconfig.json files and their moduleResolution settings:" find . -name "tsconfig.json" -exec sh -c 'echo -e "\nFile: {}"; grep -A 1 "moduleResolution" "{}" 2>/dev/null || echo "No moduleResolution found"' \;Length of output: 2873
packages/orderbook/tsconfig.json (1)
2-26
: Configuration structure is well-organizedThe configuration maintains consistency with other packages while properly structuring the compiler options and project references.
packages/react-hooks/tsconfig.json (1)
26-34
: Verify project reference cycle and build orderNew project references have been added to core, orderbook, and utils. Please ensure:
- There are no circular dependencies between packages
- The build order is correct based on these dependencies
✅ Verification successful
Project references are correctly structured
The dependency graph is acyclic and the build order is properly maintained through the project references. The changes are safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential circular dependencies in project references echo "Analyzing project references for circular dependencies:" find . -name "tsconfig.json" -exec sh -c 'echo "\nFile: {}"; cat {} | jq -r ".references[].path"' \;Length of output: 1837
packages/core/package.json (1)
13-14
: LGTM! File extension changes improve clarity.The changes to use explicit file extensions (.cjs.js and .es.js) better indicate the module format, which is a good practice for module resolution.
Let's verify the existence of these build artifacts:
Also applies to: 18-20
✅ Verification successful
Build configuration correctly matches package.json paths
The Vite configuration is properly set up to generate the exact file paths referenced in package.json:
index.cjs.js
for CommonJS (main)index.es.js
for ES modules (module)- Types are also properly configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the build artifacts exist with correct extensions # Check if the build artifacts exist fd -t f "index\.(cjs|es)\.js$" packages/core/dist/ # Verify the module type in the files echo "Checking module type in files..." rg -l "(import|export|require)" packages/core/dist/Length of output: 348
Script:
#!/bin/bash # Check build configuration and scripts echo "=== Package Scripts ===" cat packages/core/package.json | jq '.scripts' echo -e "\n=== Checking .gitignore ===" rg "dist" .gitignore echo -e "\n=== Build Config Files ===" fd -t f "(rollup|webpack|vite|tsup)\.config\.(js|ts|mjs|cjs)" packages/core/Length of output: 476
Script:
#!/bin/bash # Check Vite configuration for build outputs echo "=== Vite Config ===" cat packages/core/vite.config.ts echo -e "\n=== Package Exports ===" cat packages/core/package.json | jq '.exports'Length of output: 1863
readme.md (1)
1-3
: LGTM! Enhanced package visibility with badges.The addition of NPM badges for all packages improves discoverability and version tracking.
external: [ | ||
// Explicitly handle all dependencies as external | ||
...Object.keys(pkg.dependencies || {}), | ||
...Object.keys(pkg.devDependencies || {}), | ||
/node_modules/, // This pattern will match all node_modules | ||
], |
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.
Reevaluate including devDependencies
in external
configuration
Including devDependencies
in the external
array may not be appropriate, as these packages are used during development and not expected to be present in the production environment. This could lead to issues if any devDependencies are required at runtime but are not bundled.
Consider modifying the external
configuration:
external: [
// Explicitly handle all dependencies as external
...Object.keys(pkg.dependencies || {}),
- ...Object.keys(pkg.devDependencies || {}),
/node_modules/, // This pattern will match all node_modules
],
📝 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.
external: [ | |
// Explicitly handle all dependencies as external | |
...Object.keys(pkg.dependencies || {}), | |
...Object.keys(pkg.devDependencies || {}), | |
/node_modules/, // This pattern will match all node_modules | |
], | |
external: [ | |
// Explicitly handle all dependencies as external | |
...Object.keys(pkg.dependencies || {}), | |
/node_modules/, // This pattern will match all node_modules | |
], |
"**/*.test.ts", | ||
"**/*.spec.ts", | ||
"**/*.test.tsx", | ||
"**/*.spec.tsx", | ||
"**/*.test.js", | ||
"**/*.spec.js", | ||
"**/*.test.jsx", | ||
"**/*.spec.jsx", | ||
"**/*.d.ts" |
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.
🛠️ Refactor suggestion
Review test file include patterns
The expanded patterns now include JS/JSX files and are no longer scoped to the src/
directory. This could potentially include unwanted files from node_modules/
or other directories.
Consider scoping the patterns to the source directory:
"include": [
- "**/*.test.ts",
- "**/*.spec.ts",
- "**/*.test.tsx",
- "**/*.spec.tsx",
- "**/*.test.js",
- "**/*.spec.js",
- "**/*.test.jsx",
- "**/*.spec.jsx",
- "**/*.d.ts"
+ "src/**/*.test.{ts,tsx,js,jsx}",
+ "src/**/*.spec.{ts,tsx,js,jsx}",
+ "src/**/*.d.ts"
]
📝 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.
"**/*.test.ts", | |
"**/*.spec.ts", | |
"**/*.test.tsx", | |
"**/*.spec.tsx", | |
"**/*.test.js", | |
"**/*.spec.js", | |
"**/*.test.jsx", | |
"**/*.spec.jsx", | |
"**/*.d.ts" | |
"src/**/*.test.{ts,tsx,js,jsx}", | |
"src/**/*.spec.{ts,tsx,js,jsx}", | |
"src/**/*.d.ts" |
packages/core/README.md
Outdated
![Contributors](https://img.shields.io/github/contributors/catalogfi/garden.js.svg?style=for-the-badge) | ||
![Forks](https://img.shields.io/github/forks/catalogfi/garden.js.svg?style=for-the-badge) | ||
![Stargazers](https://img.shields.io/github/stars/catalogfi/garden.js.svg?style=for-the-badge) | ||
![Issues](https://img.shields.io/github/issues/othneildrew/Best-README-Template.svg?style=for-the-badge) |
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.
Fix the issues badge URL.
The issues badge points to "othneildrew/Best-README-Template" instead of the Garden repository.
Apply this fix:
-![Issues](https://img.shields.io/github/issues/othneildrew/Best-README-Template.svg?style=for-the-badge)
+![Issues](https://img.shields.io/github/issues/catalogfi/garden.js.svg?style=for-the-badge)
📝 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.
![Issues](https://img.shields.io/github/issues/othneildrew/Best-README-Template.svg?style=for-the-badge) | |
![Issues](https://img.shields.io/github/issues/catalogfi/garden.js.svg?style=for-the-badge) |
packages/core/README.md
Outdated
1. [Installation](#installation) | ||
2. [Setup](#setup) | ||
1. [Node](#node) | ||
2. [Browser](#browser) | ||
1. [Vite](#vite) | ||
2. [Webpack](#webpack) | ||
3. [Documentation](#documentation) | ||
4. [Contributions](#contributions) | ||
5. [License](#license) | ||
6. [Keep Updated](#keep-updated) |
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.
💡 Codebase verification
Fix incorrect and missing section references in Table of Contents
The following issues were found:
- "Contributions" link should be "Contributing" to match the actual section header
- "License" and "Keep Updated" sections don't exist in the document but are referenced in TOC
- Either add these missing sections or remove their entries from the TOC
🔗 Analysis chain
Verify the link fragments in Table of Contents.
Some link fragments (#license and #keep-updated) might be invalid as they don't correspond to actual sections in the document.
Let's verify the link fragments:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all linked sections exist in the document
# Check for section headers that should match the TOC
echo "Checking for section headers..."
rg "^##?\s+(Installation|Setup|Documentation|Contributions|License|Keep Updated)" packages/core/README.md
Length of output: 222
Script:
#!/bin/bash
# Description: Perform a more thorough check for section headers with variations
echo "Checking for section headers with variations..."
rg -i "^#{1,3}\s+(installation|setup|documentation|contributions?|contributing|license|keep.?updated)" packages/core/README.md
Length of output: 291
🧰 Tools
🪛 Markdownlint (0.37.0)
21-21: null
Link fragments should be valid
(MD051, link-fragments)
22-22: null
Link fragments should be valid
(MD051, link-fragments)
23-23: null
Link fragments should be valid
(MD051, link-fragments)
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/README.md (2)
6-6
:⚠️ Potential issueFix the issues badge URL.
The issues badge points to "othneildrew/Best-README-Template" instead of the Garden repository.
Apply this fix:
-![Issues](https://img.shields.io/github/issues/othneildrew/Best-README-Template.svg?style=for-the-badge) +![Issues](https://img.shields.io/github/issues/catalogfi/garden.js.svg?style=for-the-badge)
14-23
:⚠️ Potential issueFix incorrect and missing section references in Table of Contents
The following issues were found:
- "Contributions" link should be "Contributing" to match the actual section header
- "License" section doesn't exist in the document but is referenced in TOC
- "Keep Updated" should be "Stay updated" to match the actual section header
Apply these fixes:
-4. [Contributions](#contributions) -5. [License](#license) -6. [Keep Updated](#stay-updated) +4. [Contributing](#contributing) +5. [Stay Updated](#stay-updated)🧰 Tools
🪛 Markdownlint (0.37.0)
21-21: null
Link fragments should be valid(MD051, link-fragments)
22-22: null
Link fragments should be valid(MD051, link-fragments)
🧹 Nitpick comments (1)
packages/core/README.md (1)
173-173
: Remove trailing punctuation from headingRemove the colon from the heading to follow markdown style guidelines.
Apply this fix:
-### Top contributors: +### Top contributors🧰 Tools
🪛 Markdownlint (0.37.0)
173-173: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/README.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
packages/core/README.md
21-21: null
Link fragments should be valid
(MD051, link-fragments)
22-22: null
Link fragments should be valid
(MD051, link-fragments)
173-173: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
packages/core/README.md (1)
Line range hint
27-95
: Well-structured installation and setup instructions!The installation and setup instructions are clear, comprehensive, and include configuration examples for different environments.
|
||
```bash | ||
git clone https://github.com/catalogfi/garden.js.git | ||
cd core |
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.
Fix the directory change command
The command cd core
is incorrect as the package is located in the packages/core
directory.
Apply this fix:
-cd core
+cd garden.js/packages/core
📝 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.
cd core | |
cd garden.js/packages/core |
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
Configuration Updates
Package Management
@gardenfi/utils
,@gardenfi/react-hooks
, and@gardenfi/wallet-connectors
.Development Improvements
Dependency Management