fix: Use interface for logger #12
Conversation
WalkthroughThis update restructures the logging system by replacing an existing struct-based logger with a more flexible interface-based design. A new implementation, Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant LogIF as logger.Logger
participant Impl as slogLogger
participant Underlying as slog.Logger
App->>LogIF: Call Info/Debug/Error/Close
Note right of LogIF: LogIF is an interface
LogIF->>Impl: Delegate call to slogLogger implementation
Impl->>Underlying: Forward call to underlying slog.Logger
Underlying-->>Impl: Return logging output
Impl-->>LogIF: Operation complete
Poem
✨ Finishing Touches
🪧 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 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: 0
🧹 Nitpick comments (3)
internal/testutils/testutils.go (1)
21-35: Consider returninglogger.Loggerfrom this constructor.
Returning the concrete type here works fine, but returning the interface can sometimes make tests more flexible and decoupled from the concrete logger implementation.-func NewTestLogger(t *testing.T) *testLogger { +func NewTestLogger(t *testing.T) logger.Logger {internal/logger/logger.go (2)
29-39: Consider returning theLoggerinterface fromNewLogger.
Exposing only the interface improves abstraction and keeps code open for alternate implementations.-func NewLogger(filename string) *slogLogger { +func NewLogger(filename string) Logger {
54-58: Likewise, returningLoggerhere maintains consistency.
Returning an interface further decouples your calling code from the internal struct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
internal/logger/logger.go(3 hunks)internal/testutils/testutils.go(3 hunks)internal/tg/completion/completion.go(1 hunks)internal/tg/definition/definition.go(1 hunks)internal/tg/hover/hover.go(1 hunks)internal/tg/state.go(6 hunks)main.go(2 hunks)
🔇 Additional comments (21)
internal/tg/hover/hover.go (1)
23-23: Smooth change to use logger interface!You've transitioned from using a pointer to the logger (
*logger.Logger) to using the interface value directly (logger.Logger). This move makes the code more flexible since it can now work with any implementation of the logger interface.internal/tg/definition/definition.go (1)
24-24: Nice and consistent logger parameter updateThis change aligns with the broader effort to use the logger interface throughout the codebase. Using the interface value directly (
logger.Logger) instead of a pointer gives you more flexibility with different logger implementations while keeping the actual logging behavior the same.internal/tg/completion/completion.go (1)
13-13: Good job keeping consistency with logger interfaceJust like in the other files, you've updated the parameter type from a pointer to the interface value. This makes the code more modular and easier to test since you can now plug in different logger implementations as needed.
main.go (3)
48-48: Clean interface transition for handleMessageChanging from
*logger.Loggertologger.Loggeris a good move that aligns with the interface-based approach you're implementing. This makes your code more flexible without changing its behavior.
210-210: Consistent logger parameter updateNice work maintaining consistency with the logger interface changes throughout the codebase. This change to
writeResponsecompletes the transition to using the logger interface directly.
19-19:✅ Verification successful
Verify logger initialization with new interface
While not part of the explicit changes, make sure the
NewLoggerfunction here is updated to return the new interface implementation. This should work fine if the changes to the logger package are implemented correctly.
🏁 Script executed:
#!/bin/bash # Check the logger implementation and NewLogger function cat internal/logger/logger.go | grep -A 20 "func NewLogger"Length of output: 570
Heads-up: Logger Initialization Verified
The
NewLoggerfunction ininternal/logger/logger.gonow initializes the logger using the new interface implementation. The slog-based configuration—with its appropriate text and JSON handlers—confirms that the updated logger package changes are in effect.
- The function correctly returns a
*slogLogger, aligning with the new interface expectations.- Both cases (empty filename and file logging) demonstrate the proper setup.
Overall, nothing further is needed here.
internal/testutils/testutils.go (3)
13-19: Looks solid for ensuring interface compliance.
This assignment to_is a neat way to verify thattestLoggerindeed fulfills thelogger.Loggerinterface at compile time.
67-74: Clean and safe resource cleanup.
Checking for a non-nilcloserbefore callingClose()is a good practice.
76-89: Straightforward delegated logging methods.
These methods neatly wrap the underlying slog calls. Looks good.internal/logger/logger.go (6)
10-16: Good interface binding forslogLogger.
This confirmsslogLoggersatisfiesLoggerat compile time, avoiding surprises later.
18-23: Interface design covers essential logging needs.
DefiningClose,Debug,Info, andErroris clean and covers typical logging usage.
61-67: Clear, minimal resource cleanup.
The conditional close helps avoid nil-pointer panics.
70-72: Nicely delegated debug logging.
No extra overhead or hidden complexities—very straightforward.
75-77: Well-structured info logging.
Consistent usage of the embedded slog logger.
80-82: Straightforward error logging.
Matches the rest of the logging pattern.internal/tg/state.go (6)
28-36: Switching to an interface parameter is a welcome simplification.
Passing aroundlogger.Loggerhere helps maintain a clean layering.
38-46: Same updates forUpdateDocument.
Also a straightforward improvement by using the interface type.
48-72:updateStatenicely adopts the interface too.
Keeps the pattern consistent—no immediate issues spotted.
74-139:Hovermethod transitions smoothly to the new interface.
No logic concerns; calls remain the same, just with the interface param.
153-229:Definitionalso fits well with the interface.
Looks like the debugging logic remains intact, just with an updated param type.
247-259:TextDocumentCompletioncompletes the logger interface switch.
Having all these methods rely onlogger.Loggeris a good consistency move.
1 participant
Summary by CodeRabbit