-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Added Logging
#21
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
Conversation
|
Warning Rate limit exceeded@samtrion has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new logging project and extension methods, registers centralized package versions for logging, adds runtime fallback registration for a null logger, and adjusts test program argument handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs (1)
3-10: Sensible default command handling for empty argsUsing a local
argumentsvariable and defaulting to"build"when no args are provided makes the test console more robust and removes the “no command” edge case. The flow intoCreateDefaultBuilder(arguments)looks good.If you prefer a more compact style, this is equivalent:
var arguments = args.Length == 0 ? new[] { "build" } : args;but the current version is perfectly acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
Directory.Packages.props(1 hunks)ForgingBlazor.slnx(1 hunks)src/NetEvolve.ForgingBlazor.Logging/ApplicationBuilderExtensions.cs(1 hunks)src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj(1 hunks)src/NetEvolve.ForgingBlazor/ForgingBlazorApplicationBuilder.cs(2 hunks)tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs (1)
src/NetEvolve.ForgingBlazor/ForgingBlazorApplicationBuilder.cs (2)
ForgingBlazorApplicationBuilder(33-195)ForgingBlazorApplicationBuilder(70-75)
🪛 GitHub Check: Build & Tests / Run Tests (ubuntu-latest) / Testing .NET solution
src/NetEvolve.ForgingBlazor/ForgingBlazorApplicationBuilder.cs
[failure] 104-104:
'IServiceCollection' does not contain a definition for 'IsServiceTypeRegistered' and no accessible extension method 'IsServiceTypeRegistered' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?)
[failure] 104-104:
'IServiceCollection' does not contain a definition for 'IsServiceTypeRegistered' and no accessible extension method 'IsServiceTypeRegistered' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?)
[failure] 104-104:
'IServiceCollection' does not contain a definition for 'IsServiceTypeRegistered' and no accessible extension method 'IsServiceTypeRegistered' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?)
[failure] 104-104:
'IServiceCollection' does not contain a definition for 'IsServiceTypeRegistered' and no accessible extension method 'IsServiceTypeRegistered' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & Tests / Run Tests (windows-latest) / Testing .NET solution
- GitHub Check: Build & Tests / Run Tests (macos-latest) / Testing .NET solution
🔇 Additional comments (4)
ForgingBlazor.slnx (1)
26-26: Logging project correctly wired into solutionThe new
NetEvolve.ForgingBlazor.Loggingproject is added under/src/consistently with existing entries; no issues from a solution-structure perspective.src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj (1)
1-16: Logging project configuration is consistent with the existing solution setupThe new logging project:
- Targets the shared
$( _ProjectTargetFrameworks ).- Correctly relies on central package version management for logging and YamlDotNet.
- Exposes internals to
NetEvolve.ForgingBlazorand references the extensibility project.All of this looks coherent with the rest of the solution.
Directory.Packages.props (1)
21-24: Central logging package versions align with new logging projectAdding central versions for
Microsoft.Extensions.Loggingand.Consolematches the new logging project’sPackageReferences and keeps versioning consistent across the solution.Please just confirm these versions are compatible with your target frameworks and other dependencies in this repo.
src/NetEvolve.ForgingBlazor.Logging/ApplicationBuilderExtensions.cs (1)
15-114: Logging extension methods look well‑structured and DI‑friendlyThe
WithLoggingoverloads integrate cleanly withIApplicationBuilder:
WithLogging(this IApplicationBuilder builder)provides a sensible default (AddConsole().AddDebug()).- The configurable overload null‑checks
builder, wires logging viabuilder.Services.AddLogging(configure), and preserves fluent chaining by returning the same builder instance.This matches typical
Microsoft.Extensions.Loggingusage patterns and should compose nicely with theForgingBlazorApplicationBuilder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs (1)
3-4: Consider removing unused imports.The following imports are not referenced in this file:
System.CommandLine(line 3)System.Diagnostics.CodeAnalysis(line 4)NetEvolve.ForgingBlazor.Commands(line 6)NetEvolve.ForgingBlazor.Core.Models(line 7)NetEvolve.ForgingBlazor.Extensibility.Abstractions(line 8)NetEvolve.ForgingBlazor.Extensibility.Models(line 9)Apply this diff to remove unused imports:
-using System.CommandLine; -using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.DependencyInjection; -using NetEvolve.ForgingBlazor.Commands; -using NetEvolve.ForgingBlazor.Core.Models; -using NetEvolve.ForgingBlazor.Extensibility.Abstractions; -using NetEvolve.ForgingBlazor.Extensibility.Models; -using NetEvolve.ForgingBlazor.Services;Also applies to: 6-9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs
[error] 10-10: dotnet build failed: CS0234 The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
🪛 GitHub Check: Build & Tests / Run Tests (macos-latest) / Testing .NET solution
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
🪛 GitHub Check: Build & Tests / Run Tests (ubuntu-latest) / Testing .NET solution
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
🪛 GitHub Check: Build & Tests / Run Tests (windows-latest) / Testing .NET solution
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
[failure] 10-10:
The type or namespace name 'Services' does not exist in the namespace 'NetEvolve.ForgingBlazor' (are you missing an assembly reference?)
🔇 Additional comments (1)
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs (1)
14-15: LGTM!The implementation correctly checks if a service type is registered in the collection. The use of LINQ's
Anymethod with a type comparison is appropriate and efficient.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
==========================================
+ Coverage 89.81% 90.26% +0.45%
==========================================
Files 10 11 +1
Lines 108 113 +5
Branches 4 6 +2
==========================================
+ Hits 97 102 +5
Misses 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.