Skip to content

Extract Serilog file logging into Libraries/SerilogFileLogging - #8149

Merged
justindbaur merged 2 commits into
mainfrom
extract-serilog-file-logging
Aug 5, 2026
Merged

Extract Serilog file logging into Libraries/SerilogFileLogging#8149
justindbaur merged 2 commits into
mainfrom
extract-serilog-file-logging

Conversation

@justindbaur

Copy link
Copy Markdown
Member

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-34566

📔 Objective

Moves AddSerilogFileLogging and its tests out of Core into a new src/Libraries/SerilogFileLogging library.

This allows services that need file logging to take a direct dependency on the library without depending on Core, supporting the broader effort to decompose Core into smaller, more focused libraries (ADR-0032).

Core continues to reference the library transitively for now, so existing consumers are unaffected.

Moves AddSerilogFileLogging and its tests out of Core into a dedicated
library so services that need file logging can depend on it directly
without taking a dependency on Core.
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR extracts AddSerilogFileLogging and its tests from Core into a new src/Libraries/SerilogFileLogging library, in line with the Core break-up effort (ADR-0032). The moved LoggerFactoryExtensions.cs is behaviorally identical to the original — only XML doc comments changed — and it retains the Bit.Core.Utilities namespace, matching how the other extracted libraries handle namespaces so existing consumers stay unaffected. Core picks up Serilog.Extensions.Logging.File transitively via the new ProjectReference, and all packages.lock.json files were regenerated consistently with the package moving from Direct to Transitive. No new dependencies were introduced, so no AppSec review is triggered. Both findings below are non-blocking tech debt.

Code Review Details
  • ♻️ : New library and test projects are not registered in bitwarden-server.slnx, unlike all four sibling src/Libraries/ projects — they won't appear in IDEs and are skipped by the root dotnet format --verify-no-changes lint job
    • src/Libraries/SerilogFileLogging/SerilogFileLogging.csproj:1
  • ♻️ : TempDirectory is a verbatim copy of the still-in-use test/Core.Test/TempDirectory.cs, carrying forward a #region that .claude/CLAUDE.md explicitly forbids
    • test/Libraries/SerilogFileLogging.Test/TempDirectory.cs:1

Minor, not commented inline: the two new test files omit the UTF-8 BOM that .editorconfig (charset = utf-8-bom) specifies for *.cs and that every sibling library file has.

@justindbaur justindbaur added the t:tech-debt Change Type - Tech debt label Aug 5, 2026
Comment thread test/Libraries/SerilogFileLogging.Test/TempDirectory.cs Fixed
Comment thread test/Libraries/SerilogFileLogging.Test/TempDirectory.cs Fixed
Comment thread test/Libraries/SerilogFileLogging.Test/TempDirectory.cs Fixed
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.48%. Comparing base (b3d1eb9) to head (f27478d).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8149      +/-   ##
==========================================
+ Coverage   62.88%   67.48%   +4.60%     
==========================================
  Files        2312     2312              
  Lines      100299   100325      +26     
  Branches     9021     9022       +1     
==========================================
+ Hits        63070    67703    +4633     
+ Misses      35053    30343    -4710     
- Partials     2176     2279     +103     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@justindbaur
justindbaur requested a review from a team August 5, 2026 17:34
Comment thread src/Libraries/SerilogFileLogging/SerilogFileLogging.csproj
Comment thread test/Libraries/SerilogFileLogging.Test/TempDirectory.cs Outdated
…helper

Add SerilogFileLogging and SerilogFileLogging.Test to bitwarden-server.slnx so
both projects appear in Test Explorer and are included in dotnet format lint.

Replace the TempDirectory IDisposable helper with Directory.CreateTempSubdirectory(),
consistent with the other tests in the file, and delete the now-unused class.
@justindbaur
justindbaur requested a review from withinfocus August 5, 2026 20:31
@justindbaur
justindbaur merged commit 974840f into main Aug 5, 2026
58 checks passed
@justindbaur
justindbaur deleted the extract-serilog-file-logging branch August 5, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants