Merged
Conversation
|
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe PR restructures the UCD manifest and store system. It converts manifest file lists from strings to structured objects with name, path, and storePath properties. The ucd-store package is refactored with new context management, file operations organized into files/reports/tasks modules, and improved error handling. CLI store commands are reorganized with new flags and output formatting changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (85)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
luxass
force-pushed
the
feat/ucd-store-fixes-redo
branch
from
4228b89 to
a121117
Compare
luxass
force-pushed
the
feat/ucd-store-fixes-redo
branch
from
8874128 to
e8f3414
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
luxass
force-pushed
the
feat/ucd-store-fixes-redo
branch
3 times, most recently
from
eba2f38 to
02199e8
Compare
luxass
force-pushed
the
feat/ucd-store-fixes-redo
branch
2 times, most recently
from
8d9b3cc to
4381315
Compare
Removed the redundant `isContext` function and replaced it with `isUCDStoreInternalContext` for better clarity. Improved the `analyze` function to handle context and options more intuitively.
- Updated command tables in `store/root.ts` for clarity. - Enhanced output formatting in `store/status.ts`, `store/sync.ts`, and `store/verify.ts` to use consistent logging methods. - Refactored error handling and success messages for better user experience. - Adjusted tests in `analyze.test.ts`, `init.test.ts`, `mirror.test.ts`, `status.test.ts`, `sync.test.ts`, and `verify.test.ts` to reflect changes in output methods.
- Introduced a `normalizeApiPath` function to streamline path normalization for mirroring. - Updated the mirroring process to utilize a "merge" strategy for version handling. - Improved debug logging for better traceability during file operations. - Removed unnecessary console error logging in the file listing process.
Updated the file reporting structure in the `_mirror` function to use a `ReportFile` interface for better clarity and consistency. This change includes modifying the types for downloaded, skipped, and failed files, as well as adjusting the normalization of file paths for filtering.
Modified the assertions in the mirror tests to validate the structure of the downloaded files report, ensuring it includes both `name` and `filePath` for each file.
- Introduced custom error classes for better error management. - Updated `assertLocalStore` and `assertRemoteOrStoreDir` functions to throw specific errors. - Enhanced error reporting in `runCLI` to utilize the new `CLIError` class. - Adjusted error message formatting in `runMirrorStore` for clarity.
Updated the file access methods in the `ucd-store` package to utilize the new store subdomain (`ucd-store.ucdjs.dev`). This change simplifies the path structure by removing the `/ucd/` prefix and enhances security by preventing path traversal vulnerabilities. All related tests have been updated to reflect these changes, ensuring that file operations now correctly reference the new path format.
luxass
force-pushed
the
feat/ucd-store-fixes-redo
branch
from
4381315 to
ff4184f
Compare
- Simplified the manifest generation by introducing `ExpectedFile` schema. - Updated the `upload-manifest` endpoint to accept version as a query parameter. - Changed the tar creation to only include `manifest.json`. - Enhanced error handling and logging for better traceability. - Updated tests to reflect changes in expected file structure. This refactor improves the clarity and maintainability of the manifest handling process.
Contributor
📋 OpenAPI Schema AnalysisSummarySchema Components
Overall Status
Detailed Changes📋 Schema Components ChangesAdded Schemas (2):
🤖 This comment is automatically updated when you push new commits. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the ucd-store package architecture, reorganizing internal operations and updating the public API structure. The changes primarily involve moving from a flat operations structure to a namespaced approach with tasks and reports, updating test assertions for better type safety, and replacing the pathe dependency with a workspace-local @ucdjs/path-utils package.
Changes:
- Reorganized operations into
tasks/(sync, mirror) andreports/(analyze) directories - Updated public API:
store.getFileTree()→store.files.tree() - Replaced
pathedependency with@ucdjs/path-utils - Improved test assertions using
expect.objectContaining()for partial matching - Updated error messages from "verification" to "validation" for consistency
- Added comprehensive integration tests for Node.js and HTTP environments
Reviewed changes
Copilot reviewed 84 out of 86 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vscode/src/lib/files.ts | Updated API call from store.getFileTree() to store.files.tree() |
| pnpm-lock.yaml | Replaced pathe dependency with @ucdjs/path-utils workspace package |
| packages/ucd-store/package.json | Updated dependencies to use workspace @ucdjs/path-utils |
| packages/ucd-store/test/version-conflict.test.ts | Changed assertions to use expect.objectContaining() for partial matching |
| packages/ucd-store/test/utils/verify.test.ts | Updated function import path and error message text |
| packages/ucd-store/test/utils/lockfile.test.ts | Renamed bootstrap → initLockfile, updated assertions and error messages |
| packages/ucd-store/test/tasks/*.test.ts | New comprehensive test files for sync, mirror operations |
| packages/ucd-store/test/reports/analyze.test.ts | New test file for analyze reporting functionality |
| packages/ucd-store/test/integration/*.test.ts | New integration tests for Node.js and HTTP store implementations |
| packages/ucd-store/test/files/*.test.ts | New test files for file operations (get, list, tree) |
| packages/ucd-store/test/helpers/test-context.ts | Refactored context creation with improved type safety |
| Multiple deleted test files | Removed old test organization under operations/ and core/ |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
pkg: cli
api
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description
Summary by CodeRabbit
Release Notes
New Features
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.