refactor: migrate logger from internal-utils#551
Conversation
WalkthroughThe type alias Changes
Estimated code review effort1 (~2 minutes) Poem
✨ 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 (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/logger.ts(3 hunks)
🔇 Additional comments (2)
src/logger.ts (2)
18-18: LGTM! Debug method properly added to interface.The debug method signature is consistent with other logging methods and correctly uses the expanded
Contextstype. The placement between verbose and info follows a logical logging level hierarchy.
30-30: LGTM! Correct no-op implementation.The debug method implementation correctly provides a no-operation function that satisfies the
ILoggerinterface contract and maintains consistency with other no-op methods.
| export type Contexts = Record< | ||
| string, | ||
| Context | string | number | boolean | null | undefined | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good improvement to context flexibility, but note inconsistency with verbose method.
The expansion of Contexts to include primitive types is beneficial for usability, allowing direct primitive values rather than requiring wrapping in objects. However, there's an inconsistency: the verbose method on line 16 still uses Record<string, unknown> while other logging methods use the new Contexts type.
Consider updating the verbose method for consistency:
- verbose: (message: string, data?: Record<string, unknown>) => void
+ verbose: (message: string, context?: Contexts) => voidCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/logger.ts around lines 6 to 9 and line 16, the Contexts type has been
expanded to include primitive types for better flexibility, but the verbose
method still uses Record<string, unknown>, causing inconsistency. Update the
verbose method's parameter type to use the new Contexts type instead of
Record<string, unknown> to maintain consistency across all logging methods.
* beta: refactor: migrate runtime and waitForIntentSettlement utils (#553) refactor: migrate failover and request utils (#554) refactor: migrate publishIntent(s) function (#552) refactor: migrate logger from internal-utils (#551) refactor: migrate poaBridge utils (#550) refactor: use toError and NEP-141 storage utils from internal-utils (#548)
|
🎉 This PR is included in version 1.0.0-beta.204 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Summary by CodeRabbit
New Features
Refactor