-
-
Notifications
You must be signed in to change notification settings - Fork 6
☕ Support JSR #64
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
☕ Support JSR #64
Conversation
WalkthroughThe project now involves renaming the 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
==========================================
- Coverage 99.40% 99.40% -0.01%
==========================================
Files 9 9
Lines 670 669 -1
Branches 72 72
==========================================
- Hits 666 665 -1
Misses 4 4 ☔ View full report in Codecov by Sentry. |
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 selected for processing (1)
- deno.jsonc (1 hunks)
Additional comments: 2
deno.jsonc (2)
- 2-4: The addition of
@lambdalisue/unknownutilversion3.16.3with an export path of./mod.tsis correctly formatted for a Deno project. However, ensure that this version of the package supports the JSR features you intend to use. It's also important to verify that this package version doesn't introduce breaking changes or conflicts with other dependencies.- 1-7: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [5-14]
The configuration for tasks and imports remains unchanged and appears to be correctly set up for a Deno project. It's good practice to periodically review these configurations to ensure they align with the latest Deno features and best practices, especially after adding new dependencies.
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 selected for processing (1)
- metadata.ts (3 hunks)
Additional comments: 3
metadata.ts (3)
- 8-8: Renaming the metadata key to
__unknownutil_metadataimproves namespacing and reduces the likelihood of name collisions. This change is consistent throughout the file.- 22-22: The usage of the renamed metadata key within the
getMetadatafunction is correct and follows the new naming convention.- 45-45: The application of the new metadata key within the
setPredicateFactoryMetadatafunction is consistent with its definition and usage elsewhere in the file. This ensures uniformity in accessing metadata across different parts of the code.
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 selected for processing (2)
- deno.jsonc (1 hunks)
- metadata.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- deno.jsonc
- metadata.ts
JSR does not allow slow types. See https://jsr.io/docs/about-slow-types ``` error: TS1170 [ERROR]: A computed property name in a type literal must refer to an expression whose type is a literal type or a 'unique symbol' type. [metadataKey]: T; ~~~~~~~~~~~~~ at file:///Users/alisue/ghq/github.com/lambdalisue/deno-unknownutil/metadata.ts:9:26 TS2304 [ERROR]: Cannot find name 'metadataKey'. [metadataKey]: T; ~~~~~~~~~~~ at file:///Users/alisue/ghq/github.com/lambdalisue/deno-unknownutil/metadata.ts:10:4 Found 2 errors. ```
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: 2
Actionable comments outside the diff hunks (2)
is/factory_test.ts (2)
Line range hint
33-33: The tests are thorough and well-structured, covering a wide range of cases for each utility function. Good use of Deno's testing features and assertions.
Line range hint
33-33: The usage of utility functions within the tests demonstrates a good understanding of TypeScript's type system and the capabilities of the utility functions. Effective use of type assertions.
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (11)
- .github/workflows/jsr.yml (1 hunks)
- README.md (1 hunks)
- deno.jsonc (1 hunks)
- is/_deprecated.ts (1 hunks)
- is/_deprecated_test.ts (1 hunks)
- is/annotation_test.ts (1 hunks)
- is/core_test.ts (1 hunks)
- is/factory.ts (1 hunks)
- is/factory_test.ts (1 hunks)
- is/utility_test.ts (1 hunks)
- metadata.ts (3 hunks)
Additional comments not posted (9)
is/_deprecated.ts (1)
3-4: Ensure that the updated import paths forisObjectOfandGetMetadataare correct and align with the new structure of the project.metadata.ts (1)
8-8: RenamingmetadataKeyto__unknownutil_metadataimproves clarity. Ensure all references to this key throughout the project are updated accordingly.is/_deprecated_test.ts (1)
10-10: The change to named imports forPredicateandPredicateTypealigns with TypeScript best practices for type imports. Ensure that all usages of these types in the file are updated if necessary.is/core_test.ts (1)
6-6: Switching to named imports forPredicateis a good practice for importing types in TypeScript. Verify that this change does not affect the usage ofPredicatein the tests.README.md (1)
3-5: The updates to badge URLs and references in the README ensure that they accurately reflect the current state of the project. Double-check the URLs to ensure they are correct and functional.is/utility_test.ts (1)
10-10: The transition to named imports forPredicateandPredicateTypeis consistent with TypeScript's best practices for importing types. Confirm that the tests still function as expected after this change.is/annotation_test.ts (1)
10-10: LGTM! Usingtypefor importing types aligns with TypeScript best practices.is/factory.ts (1)
3-3: LGTM! Usingtypefor importing types aligns with TypeScript best practices.is/factory_test.ts (1)
10-10: Using thetypekeyword for importing types is a good practice for clarity and potential bundling optimizations.
| deno-version: ${{ env.DENO_VERSION }} | ||
| - name: Publish | ||
| run: | | ||
| deno run -A jsr:@david/publish-on-tag@0.1.3 |
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.
Consider pinning the version of the jsr:@david/publish-on-tag action to ensure consistent behavior across runs.
- deno run -A jsr:@david/publish-on-tag@0.1.3
+ deno run -A jsr:@david/publish-on-tag@0.1.3 # Pinning the version for consistencyCommittable 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.
| deno run -A jsr:@david/publish-on-tag@0.1.3 | |
| deno run -A jsr:@david/publish-on-tag@0.1.3 # Pinning the version for consistency |
| "name": "@core/unknownutil", | ||
| "version": "0.0.0", | ||
| "exports": "./mod.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.
The package version specified as "0.0.0" seems incorrect based on the PR objectives, which mention integrating @lambdalisue/unknownutil version 3.16.3. Ensure the version number is updated accordingly.
- "version": "0.0.0",
+ "version": "3.16.3",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.
| "name": "@core/unknownutil", | |
| "version": "0.0.0", | |
| "exports": "./mod.ts", | |
| "name": "@core/unknownutil", | |
| "version": "3.16.3", | |
| "exports": "./mod.ts", |
Summary by CodeRabbit
@lambdalisue/unknownutilversion3.16.3.