BA: restore catalogs #216
BA: restore catalogs #216
Conversation
anicioalexandre
requested review from
deboracosilveira and
priscilladeroode
as code owners
|
|
Warning Rate limit exceeded@anicioalexandre has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 9 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. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request updates dependency management across multiple packages by replacing fixed version numbers in various Changes
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/test/package.json (1)
21-22: Consider updating axios-mock-adapter to use catalog versioning.For consistency with other dependencies, consider transitioning axios-mock-adapter from caret versioning (^1.22.0) to catalog-based versioning.
packages/wagtail/package.json (1)
22-28: Consider standardizing catalog categories for all dependencies.While most dependencies use specific catalog categories (e.g.,
material-ui,storybook,graphql), some dependencies likeaxios,typescript, and@types/*use the genericcatalog:reference. This inconsistency might lead to version mismatches.Consider creating specific catalog categories for these dependencies to maintain better version control and consistency.
Also applies to: 37-84
packages/design-system/package.json (1)
61-62: Standardize dependency version management.Some dependencies use catalog references while others use fixed versions:
- Using catalog:
@gorhom/bottom-sheet,@mui/*, etc.- Using fixed versions:
@expo/vector-icons,framer-motion, etc.Consider moving all dependencies to the catalog system for consistent version management. If certain packages must use fixed versions, document the reason in a comment.
Also applies to: 74-76
packages/components/package.json (2)
84-85: Ensure consistent versioning for build dependencies.The package has a complex build process involving multiple steps. While most dependencies use catalog references, some still use fixed versions:
- Using catalog:
graphql,react-hook-form, etc.- Using fixed versions:
framer-motion,numbro,slugify, etc.Consider:
- Moving all dependencies to the catalog system for consistent version management
- Documenting any exceptions that require fixed versions
- Ensuring all build-related dependencies use catalog references to prevent build issues
Also applies to: 89-90, 97-100
1-1: Consider documenting the catalog versioning strategy.The transition to catalog-based versioning is a good move for maintainability, but the implementation could benefit from:
- Documentation explaining the catalog system and its categories
- Guidelines for when to use specific catalog categories vs. generic
catalog:- Criteria for exceptions that require fixed versions
Consider adding this documentation to the repository's README or a dedicated CONTRIBUTING guide.
packages/utils/package.json (1)
18-18: Consider updating remaining dependencies to use catalog references.For consistency with the PR objective, consider updating these dependencies to use catalog references:
- Dependencies:
- "events": "^3.3.0"
- "humps": "^2.0.1"
- "jwt-decode": "^4.0.0"
- "qs": "^6.12.1"
- "server-only": "^0.0.1"
- DevDependencies:
- "@types/humps": "^2.0.6"
- "@types/qs": "^6.9.7"
Also applies to: 21-21, 22-22, 24-24, 43-43, 49-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
packages/authentication/package.json(1 hunks)packages/components/package.json(2 hunks)packages/components/tsconfig.build.json(0 hunks)packages/config/package.json(1 hunks)packages/design-system/package.json(2 hunks)packages/design-system/tsconfig.build.json(0 hunks)packages/eslint-plugin/package.json(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/package.json(1 hunks)packages/test/package.json(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(2 hunks)
💤 Files with no reviewable changes (2)
- packages/design-system/tsconfig.build.json
- packages/components/tsconfig.build.json
✅ Files skipped from review due to trivial changes (1)
- packages/wagtail/CHANGELOG.md
🔇 Additional comments (7)
packages/config/package.json (1)
14-29: LGTM! Excellent standardization of dependency management.The transition to using
catalog:lintfor all linting-related dependencies is a well-structured change that:
- Ensures consistent versioning across all linting tools and plugins
- Simplifies dependency maintenance
- Makes it easier to update all linting dependencies as a cohesive unit
packages/eslint-plugin/package.json (2)
23-27: LGTM! Verify the catalog references.The transition to catalog references for devDependencies is consistent and well-organized:
catalog:lintfor ESLint-related typescatalog:testfor testing tools- Base catalog for TypeScript as a core dependency
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify all catalog references exist and are properly configured # Search for catalog definitions echo "Searching for catalog:test definition..." rg -g '*.json' -A 5 '"catalog:test"' | grep -v "@types/jest.*catalog:test" | grep -v "ts-node.*catalog:test" echo "Searching for base catalog definition..." rg -g '*.json' -A 5 '"catalog:"' | grep -v "typescript.*catalog:"Length of output: 22188
Catalog references verified – All catalog references are consistent across the repository.
The changes in packages/eslint-plugin/package.json follow the same pattern observed across other packages:
"catalog:lint"is correctly used for ESLint-related types."catalog:test"is applied to both testing libraries and tools like ts-node.- The base catalog reference
"catalog:"for TypeScript aligns with how core dependencies are managed elsewhere.The repository-wide search confirms that these catalog references are used uniformly. No misconfigurations were found.
19-19: LGTM! Verify the catalog reference.The transition to
catalog:lintaligns with the PR objective of restoring catalogs for centralized dependency management.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the catalog:lint reference exists and is properly configured # Search for catalog:lint definition rg -g '*.json' -A 5 '"catalog:lint"' | grep -v "eslint.*catalog:lint"Length of output: 2468
LGTM – Catalog Reference Verified
The use of
catalog:lintis consistent across the codebase (e.g., in the root and config package files), supporting the centralized dependency management objective. No discrepancies were found in its application inpackages/eslint-plugin/package.json.packages/provider/package.json (1)
16-17: LGTM! Consistent use of catalog references.The transition to catalog-based versioning for dependencies is well-structured, using appropriate catalog references:
- "catalog:" for general packages
- "catalog:react18" for React-related packages
Also applies to: 21-21, 26-28
packages/graphql/package.json (1)
16-23: Consider standardizing the catalog reference for js-cookie.While most GraphQL-related dependencies correctly use "catalog:graphql", js-cookie uses "catalog:". Consider standardizing all non-GraphQL dependencies across packages.
Let's verify the js-cookie versioning across all packages:
❌ Incorrect review comment
🏁 Script executed:
#!/bin/bash # Search for js-cookie versioning across all package.json files rg '"js-cookie":' -A 1 -g 'package.json'Length of output: 637
JS-Cookie Catalog Reference is Consistent
Our search across all package.json files shows that "js-cookie" is consistently referenced as
"catalog:"in every package (authentication, components, graphql, test, and utils). There is no discrepancy in its catalog reference that needs standardization.Likely an incorrect or invalid review comment.
packages/authentication/package.json (1)
17-22: LGTM! The catalog references are consistently applied.The changes align with the PR objective of restoring catalogs, with proper categorization:
- Dependencies use "catalog:"
- React-related dependencies use "catalog:react18"
- Testing dependencies use "catalog:test"
Let's verify that the catalog references are valid and the dependencies are properly categorized:
Also applies to: 26-26, 33-47
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that the catalog references exist and are properly categorized. # Test: Check if package.json exists in the root directory and contains catalog definitions if [ -f "package.json" ]; then echo "Checking catalog definitions in root package.json..." jq '.catalog' package.json else echo "Root package.json not found. Please ensure you're in the correct directory." fiLength of output: 172
The absence of a catalog definition in the root package.json is expected—these catalog references are defined within the package.json files in their respective packages (in this case, packages/authentication/package.json). Verifying the relevant sections (lines 17–22, 26, and 33–47) in that file confirms that the dependencies are indeed using the correct "catalog:" prefix (and similar categories where applicable).
LGTM! The catalog references are consistently applied.
packages/utils/package.json (1)
17-21: LGTM! The catalog references are consistently applied.The changes align with the PR objective of restoring catalogs, with proper categorization:
- Dependencies use "catalog:" or "catalog:react-native-core"
- React-related dependencies use "catalog:react18"
- Testing dependencies use "catalog:test"
Also applies to: 23-26, 28-30, 31-31, 34-34, 40-48, 50-57
| "name": "@baseapp-frontend/wagtail", | ||
| "description": "BaseApp Wagtail", | ||
| "version": "2.0.0", | ||
| "version": "1.0.20", |
There was a problem hiding this comment.
Version downgrade requires attention.
The package version has been downgraded from 2.0.0 to 1.0.20. This breaks semantic versioning principles and could cause compatibility issues for dependent packages.
Please verify if this downgrade was intentional. If not, consider maintaining the major version number to avoid breaking changes for consumers of this package.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
58-103: Consider adding explicit build dependencies to test jobs.The test jobs might also benefit from explicit
needs: builddependencies, similar to the lint job, to ensure they run after the build job completes. This would make the dependency chain more explicit and prevent potential race conditions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/main.yml(1 hunks)turbo.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
turbo.json (1)
35-38: LGTM! Good improvement to the build dependency chain.Adding
^buildto the lint task's dependencies ensures that all necessary artifacts are built before linting starts. This aligns with other tasks liketest:unitand maintains build integrity..github/workflows/main.yml (1)
34-37: LGTM! Good synchronization with turbo.json changes.Adding the build dependency ensures consistent behavior between local development and CI/CD pipeline.
|
Summary by CodeRabbit
These updates help maintain a more cohesive and predictable dependency and build process across the project.