Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Splat.NLog - Faster mapping of LogLevel using switch-operation #1257

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Dec 31, 2024

What kind of change does this PR introduce?

  • Minor optimization of mapping from Splat LogLevel to NLog LogLevel
  • Removes dependency on System.Collections.Immutable-nuget-package
  • Fixes minor spelling issue in XML docs for ILogger-interface

What is the current behavior?

  • Performs dictionary lookup when mapping from Splat LogLevel to NLog LogLevel
  • Has additional dependency on System.Collections.Immutable-nuget-package

What is the new behavior?

  • Performs switch-operation when mapping from Splat LogLevel to Nlog LogLevel
  • No dependency on System.Collections.Immutable-nuget-package

What might this PR break?

  • No breaking changes

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

@glennawatson
Copy link
Contributor

Thanks.

With the new switch expression definitely looks better, also no allocation overhead of the immutable dictionary.

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

Attention: Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.28%. Comparing base (1803c70) to head (d8f5984).
Report is 183 commits behind head on main.

Files with missing lines Patch % Lines
src/Splat.NLog/NLogLogger.cs 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1257      +/-   ##
==========================================
- Coverage   71.87%   71.28%   -0.60%     
==========================================
  Files          96       96              
  Lines        4483     4429      -54     
  Branches      569      561       -8     
==========================================
- Hits         3222     3157      -65     
- Misses       1074     1088      +14     
+ Partials      187      184       -3     

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

@glennawatson glennawatson merged commit ddef2ed into reactiveui:main Dec 31, 2024
2 of 3 checks passed
@snakefoot
Copy link
Contributor Author

snakefoot commented Dec 31, 2024

Really sorry but I was a little too quick. I read some more about Enum.GetValues() and it will allocate a new array on every call. So created #1258

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants