Conversation
🦋 Changeset detectedLatest commit: d761237 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change relocates filter-related functionality, including Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant UCD-Store
participant Utils
CLI->>Utils: import PRECONFIGURED_FILTERS
UCD-Store->>Utils: import createPathFilter, FilterFn
UCD-Store->>Utils: import PRECONFIGURED_FILTERS
UCD-Store-->>Utils: uses filter functions/types from utils
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/ucd-store/src/local.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/cli/src/cmd/download.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/ucd-store/src/remote.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
github-actions
bot
added
pkg: cli
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/kind-brooms-drum.md (1)
7-7: Fix grammatical error in changeset description.The apostrophe in "filter's" is incorrect. It should be "filters" (plural) not possessive.
Apply this diff:
-move filter's to utils pkg +move filters to utils pkgpackages/utils/src/index.ts (1)
18-19: Expose filter utilities and types
ExportingcreatePathFilter,PRECONFIGURED_FILTERS,FilterFn, andFilterOptionscentralizes your filtering API. Ensure that the implementations infilter.tscome with thorough unit tests in this package.
📜 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 (12)
.changeset/kind-brooms-drum.md(1 hunks)packages/cli/src/cmd/download.ts(1 hunks)packages/ucd-store/package.json(0 hunks)packages/ucd-store/src/download.ts(1 hunks)packages/ucd-store/src/index.ts(0 hunks)packages/ucd-store/src/local.ts(1 hunks)packages/ucd-store/src/remote.ts(1 hunks)packages/ucd-store/test/fs-interface.test.ts(1 hunks)packages/ucd-store/test/remote.test.ts(1 hunks)packages/utils/package.json(1 hunks)packages/utils/src/filter.ts(1 hunks)packages/utils/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/ucd-store/package.json
- packages/ucd-store/src/index.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.074Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
packages/ucd-store/src/remote.ts (1)
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.074Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
packages/ucd-store/src/local.ts (1)
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.074Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
packages/ucd-store/src/download.ts (1)
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.074Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
🔇 Additional comments (9)
packages/ucd-store/src/remote.ts (1)
5-5: LGTM! Clean refactoring of import source.The import source change from
"./filter"to"@ucdjs/utils"is consistent with moving filter functionality to a shared utils package. The functionality remains identical.packages/ucd-store/test/remote.test.ts (1)
5-5: LGTM! Test import updated consistently with refactoring.The import source change for
PRECONFIGURED_FILTERSfrom"@ucdjs/ucd-store"to"@ucdjs/utils"aligns with the filter functionality relocation. Test logic remains unaffected.packages/ucd-store/src/download.ts (1)
4-4: LGTM! Import refactoring maintains functionality and documentation accuracy.The import source change from
"./filter"to"@ucdjs/utils"is consistent with the filter functionality migration. The JSDoc comments referencing picomatch format remain accurate sincecreatePathFiltercontinues to use picomatch internally.packages/ucd-store/src/local.ts (1)
6-6:Details
✅ Verification successful
LGTM! Final import update completes the refactoring.
The import source change from
"./filter"to"@ucdjs/utils"completes the consistent migration of filter functionality to the utils package. All functionality is preserved.Let me verify that the dependency migration is complete:
🏁 Script executed:
#!/bin/bash # Description: Verify the filter functionality migration is complete # Expected: No remaining references to local filter imports, picomatch dependency moved to utils echo "=== Checking for remaining local filter imports ===" rg -n "from [\"']\.\/filter[\"']" --type ts echo -e "\n=== Checking package.json dependencies ===" echo "ucd-store package.json:" cat packages/ucd-store/package.json | jq '.dependencies.picomatch // empty' echo -e "\nutils package.json:" cat packages/utils/package.json | jq '.dependencies.picomatch // empty' echo -e "\n=== Verifying utils package exports filter functionality ===" cat packages/utils/src/index.ts | rg -A 5 -B 5 "(createPathFilter|PRECONFIGURED_FILTERS|FilterFn)"Length of output: 1095
Migration Verified: Filter functionality migration complete
- No remaining imports from
./filterinpackages/ucd-store/src/local.tsor elsewhere.packages/ucd-store/package.jsonno longer listspicomatchas a dependency.packages/utils/package.jsoncorrectly depends onpicomatch.packages/utils/src/index.tsexportscreatePathFilterandFilterFnfrom./filter, preserving all functionality.LGTM! Approving these refactoring changes.
packages/utils/src/filter.ts (1)
29-30: Good documentation improvement!Adding the import statement to the example clarifies how to use the function after it was moved to the utils package. This helps users understand the correct import path.
packages/cli/src/cmd/download.ts (1)
6-7: Import changes look correct for the refactoring.The separation is appropriate -
PRECONFIGURED_FILTERSnow comes from@ucdjs/utilswhile thedownloadfunction remains in@ucdjs/ucd-store. This aligns with the goal of moving filter-related utilities to the utils package.packages/ucd-store/test/fs-interface.test.ts (1)
24-33: Verify if this mocking change is related to the filter refactoring.This change from static to dynamic mocking for fs-extra appears unrelated to moving filter functionality from ucd-store to utils package. While the implementation looks correct, it seems like it might belong in a separate PR focused on test improvements.
Can you clarify if this mocking strategy change is necessary for the filter refactoring or if it's an unrelated improvement that was bundled with this PR?
packages/utils/package.json (2)
38-40: Add picomatch runtime dependency
Includingpicomatchhere is necessary to support the relocated filter logic in this package.
43-43: Add TypeScript definitions for picomatch
Providing@types/picomatchensures proper typing support for the new dependency during development and type checks.
luxass
force-pushed
the
luxass/refactor-move-filter-functionality-from-ucd-store-to-utils-package
branch
from
3ba7f0e to
d761237
Compare
luxass
deleted the
luxass/refactor-move-filter-functionality-from-ucd-store-to-utils-package
branch
1 participant
This PR partially resolves #48
Summary by CodeRabbit