Skip to content

Refactor DisplayManager to remove logging dependency#1627

Merged
hovancik merged 1 commit into
trunkfrom
dm-di
Aug 7, 2025
Merged

Refactor DisplayManager to remove logging dependency#1627
hovancik merged 1 commit into
trunkfrom
dm-di

Conversation

@hovancik
Copy link
Copy Markdown
Owner

@hovancik hovancik commented Aug 7, 2025

No description provided.

@hovancik hovancik requested a review from Copilot August 7, 2025 07:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactors the DisplayManager class to directly import and use the logging module instead of accepting it as a constructor dependency, simplifying the class interface.

  • Removed log parameter from DisplayManager constructor
  • Added direct import of electron-log/main.js to DisplayManager
  • Updated all test cases to reflect the simplified constructor

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
app/utils/displayManager.js Imports electron-log directly and removes log parameter from constructor
app/main.js Removes log parameter when instantiating DisplayManager
test/displayManager.js Updates all test cases to use simplified constructor and removes logging mock setup

Comment on lines +5 to 6
constructor (settings) {
this.settings = settings
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from dependency injection to direct import reduces testability and makes the DisplayManager class tightly coupled to the electron-log module. Consider keeping the log parameter optional to maintain flexibility for testing while providing a default import.

Suggested change
constructor (settings) {
this.settings = settings
constructor (settings, logInstance = log) {
this.settings = settings
this.log = logInstance

Copilot uses AI. Check for mistakes.
@hovancik hovancik merged commit 0186556 into trunk Aug 7, 2025
9 checks passed
@hovancik hovancik deleted the dm-di branch August 7, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants