feat: add ILogger interface, LogLevel, and ILoggerOptions#12
feat: add ILogger interface, LogLevel, and ILoggerOptions#12
Conversation
Domain-layer logging contracts with zero dependencies. Provides log levels (trace through fatal), child logger support, and configuration options for file/console output. GIT-33 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR introduces a new domain interface file ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/domain/interfaces/ILogger.ts`:
- Around line 10-16: Change the ILoggerOptions interface so consumers don’t need
to pass every setting: make the properties optional (e.g., level?: LogLevel;
logDir?: string; enableConsole?: boolean; enableFile?: boolean; retentionDays?:
number) and ensure callers/implementations (e.g., where createLogger or any
Logger implementation consumes ILoggerOptions) merge with sensible defaults
(like level='info', logDir='.git-mem/logs', enableConsole=false,
enableFile=true, retentionDays=7) using an object spread or similar so missing
fields are filled in at runtime.
| export interface ILoggerOptions { | ||
| level: LogLevel; | ||
| logDir: string; // Default: '.git-mem/logs' | ||
| enableConsole: boolean; // Default: false | ||
| enableFile: boolean; // Default: true | ||
| retentionDays: number; // Default: 7 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making properties optional for ergonomic configuration.
All properties are currently required, meaning implementers/consumers must always specify every option. Consider whether some properties (those with sensible defaults noted in comments) should be optional:
export interface ILoggerOptions {
level?: LogLevel; // Default: 'info' (common convention)
logDir?: string; // Default: '.git-mem/logs'
enableConsole?: boolean; // Default: false
enableFile?: boolean; // Default: true
retentionDays?: number; // Default: 7
}This allows simpler instantiation: createLogger({}) vs requiring all fields. The infrastructure implementation can apply defaults via { ...defaults, ...options }.
🤖 Prompt for AI Agents
In `@src/domain/interfaces/ILogger.ts` around lines 10 - 16, Change the
ILoggerOptions interface so consumers don’t need to pass every setting: make the
properties optional (e.g., level?: LogLevel; logDir?: string; enableConsole?:
boolean; enableFile?: boolean; retentionDays?: number) and ensure
callers/implementations (e.g., where createLogger or any Logger implementation
consumes ILoggerOptions) merge with sensible defaults (like level='info',
logDir='.git-mem/logs', enableConsole=false, enableFile=true, retentionDays=7)
using an object spread or similar so missing fields are filled in at runtime.
|
Closing — this work (ILogger interface) was already delivered via PR #18 (branch |
Summary
src/domain/interfaces/ILogger.tswith domain-layer logging contractsLogLeveltype: trace, debug, info, warn, error, fatalILoggerOptionsinterface: level, logDir, enableConsole, enableFile, retentionDaysILoggerinterface: level methods, child logger support,isLevelEnabled()Fixes GIT-33
Test plan
npm run buildcompiles cleanlynpm test— all 135 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit