Skip to content

feat!: migrate NetworkClient from Serilog.ILogger to Microsoft.Extensions.Logging.ILogger#134

Merged
erwan-joly merged 1 commit intomasterfrom
refactor/ilogger-generic
Apr 24, 2026
Merged

feat!: migrate NetworkClient from Serilog.ILogger to Microsoft.Extensions.Logging.ILogger#134
erwan-joly merged 1 commit intomasterfrom
refactor/ilogger-generic

Conversation

@erwan-joly
Copy link
Copy Markdown
Contributor

@erwan-joly erwan-joly commented Apr 24, 2026

Summary

  • NetworkClient ctor now takes Microsoft.Extensions.Logging.ILogger (non-generic) instead of Serilog.ILogger. All other files in this repo were already on MEL ILogger<T>; only NetworkClient remained.
  • Kept non-generic because NetworkClient is a base class that derived types (e.g. ClientSession) inherit from; the generic ILogger<Derived> implicitly converts to non-generic ILogger.
  • Bumps version to 8.0.0 (breaking change).

Test plan

  • dotnet test green.

Refs NosCoreIO/NosCore#1607.

Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • NosCore.Networking package version bumped to 8.0.0.

…ions.Logging.ILogger

BREAKING CHANGE: NetworkClient ctor now takes
Microsoft.Extensions.Logging.ILogger (non-generic) instead of
Serilog.ILogger. Derived classes can pass their ILogger<Derived>
directly because the generic variant extends the non-generic.
Bumps to 8.0.0.

Refs NosCoreIO/NosCore#1607.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ce827638-e26c-4d88-a5d7-6d9004b79a41

📥 Commits

Reviewing files that changed from the base of the PR and between cc45503 and c11c6c7.

📒 Files selected for processing (2)
  • src/NosCore.Networking/NetworkClient.cs
  • src/NosCore.Networking/NosCore.Networking.csproj

Walkthrough

Migrates logging calls from Serilog-specific methods to Microsoft.Extensions.Logging extension methods in NetworkClient.cs, replacing Information, Error, and Warning calls with LogInformation, LogError, and LogWarning respectively. Package version incremented to 8.0.0.

Changes

Cohort / File(s) Summary
Logging Framework Migration
src/NosCore.Networking/NetworkClient.cs
Replaces Serilog logger method calls with Microsoft.Extensions.Logging extension methods; updates using statements accordingly. Logging behavior and message mappings remain unchanged.
Version Update
src/NosCore.Networking/NosCore.Networking.csproj
Bumps package version from 7.2.0 to 8.0.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: migrating NetworkClient from Serilog.ILogger to Microsoft.Extensions.Logging.ILogger, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/ilogger-generic

Comment @coderabbitai help to get the list of available commands and usage tips.

@erwan-joly erwan-joly merged commit f09e8af into master Apr 24, 2026
4 checks passed
@erwan-joly erwan-joly deleted the refactor/ilogger-generic branch April 24, 2026 05:33
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.

1 participant