Skip to content

refactor: remove duplicate console encoding and service locator pattern#130

Merged
mcj-coder merged 1 commit intomainfrom
refactor/129-remove-duplicate-console-encoding
Jan 16, 2026
Merged

refactor: remove duplicate console encoding and service locator pattern#130
mcj-coder merged 1 commit intomainfrom
refactor/129-remove-duplicate-console-encoding

Conversation

@mcj-coder
Copy link
Copy Markdown
Contributor

@mcj-coder mcj-coder commented Jan 16, 2026

Summary

  • Removed duplicate Console.OutputEncoding/InputEncoding from Application.cs (already set in Program.cs bootstrap)
  • Replaced Service Locator pattern with constructor injection for IEnvironmentService
  • Application now receives all dependencies through constructor, improving testability

Changes

  • Application.cs: Removed duplicate encoding calls, removed IServiceProvider dependency, inject IEnvironmentService directly
  • ProgramTests.cs: Updated to use direct IEnvironmentService mock instead of IServiceProvider

Closes #129

Test Plan

  • All unit tests pass (163 tests)
  • All system tests pass (25 tests)
  • Build succeeds

🤖 Generated with Claude Code

@mcj-coder @claude
refactor: remove duplicate console encoding and service locator pattern
90414bf 
Changes:
1. Removed duplicate Console.OutputEncoding/InputEncoding from Application.cs
   - Encoding is already set in Program.cs bootstrap before DI
   - Application.cs should use IConsoleService for console operations

2. Replaced Service Locator pattern with constructor injection
   - Inject IEnvironmentService directly instead of via IServiceProvider
   - Application now receives all dependencies through constructor
   - Improves testability and follows DI best practices

Refs: #129

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
martincjarvis
martincjarvis approved these changes Jan 16, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@martincjarvis martincjarvis left a comment

Choose a reason for hiding this comment

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

LGTM! Good refactoring:

  1. Duplicate encoding removal - Correctly identified that Console encoding was set twice. Keeping it in Program.cs bootstrap is appropriate since encoding must be set before any console output.

  2. Service Locator elimination - Injecting IEnvironmentService directly through the constructor follows DI best practices and improves testability. The test updates correctly reflect the new pattern.

Changes are minimal, focused, and tests pass.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 16, 2026



Fails
🚫 PR must have auto-merge enabled. Enable via PR settings → "Enable auto-merge".

This ensures PRs are merged automatically once all checks pass.
🚫 PR body must contain an issue reference.

Add one of the following to your PR description:

This ensures traceability between commits and issues.
Warnings
⚠️

No plan file found for this PR. If this work has a plan, ensure it is updated.

Expected location: docs/plans/YYYY-MM-DD-<topic>-design.md with frontmatter issue: '#129'
Messages
📖 Remember: This PR will be squash-merged. Ensure the PR title follows conventional commits format.

No Plan File Detected

This PR does not modify any plan files. If this work is tracked by a plan, please update the relevant plan file in docs/plans/.

Generated by 🚫 dangerJS against 90414bf

@mcj-coder mcj-coder enabled auto-merge (squash) January 16, 2026 16:22
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 16, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/McjCoderOrg.ClaudeAutoResume/Application.cs 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@mcj-coder mcj-coder merged commit d280409 into main Jan 16, 2026
14 of 15 checks passed
@mcj-coder mcj-coder deleted the refactor/129-remove-duplicate-console-encoding branch January 16, 2026 16:27
This was referenced Jan 16, 2026
Closed
Merged
mcj-coder added a commit that referenced this pull request Jan 16, 2026
@mcj-coder @claude
fix(ci): add --fail-on-errors to danger ci command (#132)
291199c 
## Summary
- Add `--fail-on-errors` flag to `danger ci` command in workflow
- Without this flag, workflow passes even when Danger finds rule
violations

## Problem
Two separate checks were created:
1. **Check Run** ("Danger PR Validation") - Always passed if workflow
completed
2. **Status Context** ("Danger") - Showed actual failures

This allowed PR #130 to merge despite Danger showing failures.

## Fix
```diff
- run: npx danger ci
+ run: npx danger ci --fail-on-errors
```

Now the workflow will fail when Danger finds issues, blocking the merge.

Closes: #131

## Test Plan
- [x] Verify workflow syntax is valid
- [ ] Test on a PR with Danger violations to confirm it blocks merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martincjarvis martincjarvis martincjarvis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove duplicate Console encoding from Application.cs

2 participants

@mcj-coder @martincjarvis