-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: First Release NetEvolve.ForgingBlazor.Extensibility
#28
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
WalkthroughThis pull request refactors the ForgingBlazor framework by renaming core builder classes, introducing new interfaces for blog segment configuration and generic validation contexts, restructuring validation infrastructure, adding content options configuration classes, migrating command-line options to the Extensibility namespace, and updating dependent code across tests and extensions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Areas requiring extra attention:
Poem
Pre-merge checks✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
==========================================
+ Coverage 84.02% 84.70% +0.68%
==========================================
Files 16 4 -12
Lines 169 85 -84
Branches 9 2 -7
==========================================
- Hits 142 72 -70
+ Misses 24 13 -11
+ Partials 3 0 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/NetEvolve.ForgingBlazor/Application.cs (1)
15-56: Add null-guards (docs say non-null) and consider reducing constructor visibility.
Right nowargs/serviceProvidercan be null and will fail later; also the type isinternal, so the constructor beingpublicis a bit odd.- public Application(string[] args, IServiceProvider serviceProvider) + internal Application(string[] args, IServiceProvider serviceProvider) { + ArgumentNullException.ThrowIfNull(args); + ArgumentNullException.ThrowIfNull(serviceProvider); _args = args; _serviceProvider = serviceProvider; }src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs (1)
52-57: Stub implementation with unused parameters and services.The
ExecuteAsyncmethod has several issues:
- The
cancellationTokenparameter is unused- Line 54 creates a
ServiceCollection, transfers services, but discards the result- The method returns a hardcoded
0without performing any actual build operationIf this is intentional stub code for the first release, consider adding a TODO comment. Otherwise, implement the actual build logic or remove the unused code.
Would you like me to help generate a more complete implementation or add appropriate TODO comments to track the pending work?
Apply this diff to at least use the cancellationToken parameter:
- private Task<int> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken) + private Task<int> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { + // TODO: Implement actual build logic _ = new ServiceCollection().TransferAllServices(_serviceProvider); return Task.FromResult(0); }src/NetEvolve.ForgingBlazor/Commands/CommandCreate.cs (1)
44-54:ExecuteAsyncis effectively a no-op (always returns 0) and ignores inputs/cancellation.
If this is intentional for the “first release”, please add a short comment/TODO; otherwise, this likely regressed the command’s behavior (no page creation, no output/errors). Also consider honoringcancellationTokenand readingparseResultoption values (e.g.,parseResult.GetValueForOption(ProjectPath)).src/NetEvolve.ForgingBlazor/Commands/CommandExample.cs (1)
46-57:ExecuteAsynccurrently does no work and ignores cancellation/args.
As written, this command will always succeed without generating the example structure. If that’s intentional, add a TODO/comment; if not, this should be implemented (and useparseResultvalues +cancellationToken).src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (1)
70-91: Constructor contract mismatch: docs sayargscannot be null, but no guard.
Given this is public API, add anArgumentNullException.ThrowIfNull(args);to match the XML docs and avoid later NREs.public ApplicationBuilder(string[] args) { + ArgumentNullException.ThrowIfNull(args); _args = args; Services = new ServiceCollection(); }src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs (1)
105-116: Minor: exception message still says “Default pages” but API is “DefaultContent”.
Consider aligning wording to reduce confusion for consumers.- "Default pages have already been registered. Multiple registrations are not allowed." + "Default content has already been registered. Multiple registrations are not allowed."
🧹 Nitpick comments (8)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyTags.cs (1)
10-20: Rename looks consistent; consider a compatibility alias only if there were pre-release consumers.
Given this is “first release”, this is likely fine as-is. If anything outside this repo already referencedIPagePropertyTags, consider temporarily keeping the old name as an[Obsolete]interface that inherits the new one (to ease migration) and remove in the next major.tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs (1)
17-19: Variable name shadows type name.The variable name
Application(capital A) shadows the type name, which can lead to confusion and reduces code clarity. Consider using a lowercase or more descriptive name.Apply this diff to use a conventional variable name:
- if (app is Application Application) + if (app is Application application) { - Application.InvocationConfiguration = new() { Output = output }; + application.InvocationConfiguration = new() { Output = output }; }src/NetEvolve.ForgingBlazor.Extensibility/README.md (1)
223-223: Minor markdown style suggestion.The closing credit line uses bold emphasis instead of a heading. While this is a minor stylistic choice, consider using a heading for consistency with markdown best practices.
Apply this diff if you want to address the markdownlint hint:
-**Made with ❤️ by the NetEvolve Team** +## Made with ❤️ by the NetEvolve TeamAlternatively, keep it as-is if you prefer the current styling—this is purely a stylistic preference.
src/NetEvolve.ForgingBlazor/Options/BlogContentOptions.cs (1)
18-54: LGTM! Blog-specific options with reasonable defaults.The class extends the base options appropriately with blog pagination configuration. The defaults are sensible (5 items, simple "{0}" format).
Optional suggestion: Consider adding validation or documentation for
PaginationDisplayCountto clarify expected value ranges (e.g., must be ≥ 1), though runtime validation at usage sites may already handle this.src/NetEvolve.ForgingBlazor/Commands/CommandCreate.cs (1)
5-9: Nice consolidation of CLI options viaCommandOptions; consider scoping imports to what’s used.
The switch tostatic CommandOptionsand addingProjectPath/OutputPathmakes the command surface consistent with the shared options.Also applies to: 38-42
src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (1)
274-286: Validation is a bit fragile: comparingImplementationType.Nameto an open-generic type name.
Prefer checking the generic type definition whenImplementationTypeis generic, to avoid false negatives/positives.- if ( - !Services.Any(x => - x.ServiceType == _typeContentRegistration - && x.ImplementationType?.Name.Equals(_typeDefaultContentRegistration.Name, StringComparison.Ordinal) - == true - ) - ) + if (!Services.Any(x => + x.ServiceType == _typeContentRegistration + && x.ImplementationType is { IsGenericType: true } t + && t.GetGenericTypeDefinition() == _typeDefaultContentRegistration)) { throw new InvalidOperationException( $"No content registration found. Please configure default pages using {nameof(ApplicationBuilderExtensions.AddDefaultContent)}() or register custom content." ); }src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
60-66: Confirm intended boolean-option semantics (ZeroOrOnevs flag-only).
If these are meant to be simple flags,Arity = ArgumentArity.Zerois typically less confusing; current config supports--include-drafts falsewhich may be intentional—worth confirming for CLI UX consistency.Also applies to: 96-102, 114-120
src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs (1)
81-99: Keyed DI usage: ensure target framework + resolution model match the “multiple validations” remark.
AddKeyedSingletonrequires .NET 8+ and multiple registrations per key imply the runtime must enumerate keyed services (vs expecting a single keyed service).
📜 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 (54)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IBlogSegmentBuilder.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IContentRegister.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyAuthor.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyCategories.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyPublishedOn.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySeries.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySitemap.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySummary.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyTags.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidation.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidationContext.cs(2 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/Models/BlogPostBase.cs(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/NetEvolve.ForgingBlazor.Extensibility.csproj(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/README.md(1 hunks)src/NetEvolve.ForgingBlazor.Extensibility/ServiceCollectionExtensions.cs(1 hunks)src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj(1 hunks)src/NetEvolve.ForgingBlazor/Application.cs(3 hunks)src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs(12 hunks)src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs(3 hunks)src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs(1 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs(3 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandCli.cs(3 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandCreate.cs(3 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandExample.cs(4 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandOptions.cs(0 hunks)src/NetEvolve.ForgingBlazor/Commands/CommandServe.cs(3 hunks)src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csproj(1 hunks)src/NetEvolve.ForgingBlazor/Options/BlogContentOptions.cs(1 hunks)src/NetEvolve.ForgingBlazor/Options/ContentOptionsBase.cs(1 hunks)src/NetEvolve.ForgingBlazor/Options/DefaultContentOptions.cs(1 hunks)src/NetEvolve.ForgingBlazor/Options/SegmentContentOptions.cs(1 hunks)src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs(2 hunks)src/NetEvolve.ForgingBlazor/Services/ContentRegister.cs(1 hunks)src/NetEvolve.ForgingBlazor/Services/ContentRegistration.cs(1 hunks)src/NetEvolve.ForgingBlazor/Services/DefaultContentRegistration.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandBuildTests.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Helper.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Unit/ApplicationBuilderTests.cs(2 hunks)
💤 Files with no reviewable changes (1)
- src/NetEvolve.ForgingBlazor/Commands/CommandOptions.cs
🧰 Additional context used
🧬 Code graph analysis (19)
src/NetEvolve.ForgingBlazor/Options/SegmentContentOptions.cs (1)
src/NetEvolve.ForgingBlazor/Options/ContentOptionsBase.cs (1)
ContentOptionsBase(26-52)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IBlogSegmentBuilder.cs (3)
src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs (4)
IBlogSegmentBuilder(35-36)IBlogSegmentBuilder(69-79)ISegmentBuilder(141-142)ISegmentBuilder(175-176)src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs (2)
IBlogSegmentBuilder(92-99)ISegmentBuilder(111-112)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs (1)
ISegmentBuilder(50-50)
src/NetEvolve.ForgingBlazor/Application.cs (1)
src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (1)
IApplication(117-128)
tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs (2)
src/NetEvolve.ForgingBlazor/Application.cs (3)
ValueTask(79-88)Application(16-89)Application(52-56)src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (2)
ApplicationBuilder(36-297)ApplicationBuilder(86-91)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidation.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidationContext.cs (2)
IValidationContext(51-56)IValidationContext(81-86)
src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs (3)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IBlogSegmentBuilder.cs (1)
IBlogSegmentBuilder(40-40)src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs (6)
IBlogSegmentBuilder(35-36)IBlogSegmentBuilder(69-79)IApplicationBuilder(88-89)IApplicationBuilder(100-118)ISegmentBuilder(141-142)ISegmentBuilder(175-176)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs (1)
ISegmentBuilder(50-50)
src/NetEvolve.ForgingBlazor/Options/DefaultContentOptions.cs (1)
src/NetEvolve.ForgingBlazor/Options/ContentOptionsBase.cs (1)
ContentOptionsBase(26-52)
src/NetEvolve.ForgingBlazor/Commands/CommandCreate.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
src/NetEvolve.ForgingBlazor/Commands/CommandServe.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
src/NetEvolve.ForgingBlazor.Extensibility/ServiceCollectionExtensions.cs (1)
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs (2)
ServiceCollectionExtensions(13-83)IServiceCollection(36-56)
tests/NetEvolve.ForgingBlazor.Tests.Integration/Helper.cs (1)
src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (2)
ApplicationBuilder(36-297)ApplicationBuilder(86-91)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs (2)
src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs (2)
ISegmentBuilder(141-142)ISegmentBuilder(175-176)src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs (1)
ISegmentBuilder(111-112)
src/NetEvolve.ForgingBlazor/Commands/CommandExample.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs (4)
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IBlogSegmentBuilder.cs (1)
IBlogSegmentBuilder(40-40)src/NetEvolve.ForgingBlazor/Builders/BlogSegmentBuilder.cs (2)
IBlogSegmentBuilder(92-99)ISegmentBuilder(111-112)src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (3)
IApplicationBuilder(163-167)IApplicationBuilder(215-220)IApplicationBuilder(257-257)src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs (1)
ISegmentBuilder(50-50)
src/NetEvolve.ForgingBlazor/Commands/CommandCli.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
src/NetEvolve.ForgingBlazor/Options/BlogContentOptions.cs (1)
src/NetEvolve.ForgingBlazor/Options/ContentOptionsBase.cs (1)
ContentOptionsBase(26-52)
tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs (1)
src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (2)
ApplicationBuilder(36-297)ApplicationBuilder(86-91)
tests/NetEvolve.ForgingBlazor.Tests.Unit/ApplicationBuilderTests.cs (1)
src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs (2)
ApplicationBuilder(36-297)ApplicationBuilder(86-91)
🪛 LanguageTool
src/NetEvolve.ForgingBlazor.Extensibility/README.md
[grammar] ~17-~17: Ensure spelling is correct
Context: ...ssly with the ForgingBlazor application lifecycle * Define Custom Segments: Implement spe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~18-~18: Ensure spelling is correct
Context: ...d builders for content organization and navigation * Integrate Services: Transfer and mana...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~19-~19: Ensure spelling is correct
Context: ...nage services across application scopes efficiently ## Key Features ### Abstraction Layer Pr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~23-~23: Ensure spelling is correct
Context: ...ently ## Key Features ### Abstraction Layer Provides a comprehensive set of interfac...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~27-~27: Ensure spelling is correct
Context: ...): Core application initialization and execution * **Content Management** (IContentRegister...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~28-~28: Ensure spelling is correct
Context: ...ntRegistration): Content discovery and registration * **Validation Framework** (IValidation`, ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~29-~29: Ensure spelling is correct
Context: ...ionContext`): Content and configuration validation * Page Properties: Modular interfaces f...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~31-~31: Ensure spelling is correct
Context: ...`): Content organization and navigation structure ### Base Models Robust foundation classes ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~37-~37: Ensure spelling is correct
Context: ...le, link title) with YAML serialization support * BlogPostBase: Extends PageBase with blogging-spec...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~40-~40: Ensure spelling is correct
Context: ..., and tagging capabilities ### Service Management Advanced dependency injection utilities:...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~44-~44: Ensure spelling is correct
Context: ...rvice providers while maintaining scope integrity * Startup Isolation: Automatic filterin...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~57-~57: Ensure spelling is correct
Context: ...ASP.NET functionality and web framework support * System.CommandLine: Command-line inte...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~69-~69: Ensure spelling is correct
Context: ...` ## Usage ### Creating a Custom Page Type csharp using NetEvolve.ForgingBlazor.Extensibility.Models; using NetEvolve.ForgingBlazor.Extensibility.Abstractions; public record CustomPage : PageBase, IPropertySummary { public string? Summary { get; set; } // Add custom properties specific to your page type public string? CustomField { get; set; } } ### Implementing a Custom Blog Post ```csh...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~84-~84: Ensure spelling is correct
Context: ...} } ### Implementing a Custom Blog Post csharp using NetEvolve.ForgingBlazor.Extensibility.Models; using NetEvolve.ForgingBlazor.Extensibility.Abstractions; public record TutorialPost : BlogPostBase, IPropertySeries, IPropertyCategories { public string? Series { get; set; } public IReadOnlySet? Categories { get; set; } // Tutorial-specific properties public TimeSpan EstimatedDuration { get; set; } public string DifficultyLevel { get; set; } = "Beginner"; } ### Building a Content Validator csharp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~101-~101: Ensure spelling is correct
Context: ...eginner"; } ### Building a Content Validator csharp using NetEvolve.ForgingBlazor.Extensibility.Abstractions; public sealed class CustomPageValidator : IValidation { public ValueTask ValidateAsync( IValidationContext context, CancellationToken cancellationToken = default) { // Implement validation logic if (string.IsNullOrWhiteSpace(context.Content)) { context.AddError("Content cannot be empty"); return ValueTask.FromResult(false); } return ValueTask.FromResult(true); } } ### Implementing a Content Registrar cs...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~124-~124: Ensure spelling is correct
Context: ... } } ### Implementing a Content Registrar csharp using NetEvolve.ForgingBlazor.Extensibility.Abstractions; public sealed class CustomContentRegister : IContentRegister { public async ValueTask RegisterAsync( IContentRegistration registration, CancellationToken cancellationToken = default) { // Discover and register custom content var pages = await DiscoverPagesAsync(cancellationToken); foreach (var page in pages) { registration.RegisterPage(page); } } private async ValueTask<IEnumerable> DiscoverPagesAsync( CancellationToken cancellationToken) { // Implementation for content discovery await Task.CompletedTask; return Enumerable.Empty(); } } ``` ### Transferring Services Between Providers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~175-~175: Ensure spelling is correct
Context: ...r, IPropertyTags, etc.) for metadata composition 3. **Lifecycle Hooks**: Implement IApplicat...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~176-~176: Ensure spelling is correct
Context: ...ation` for custom application execution logic 4. Validation Pipeline: Add validators t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~177-~177: Ensure spelling is correct
Context: ...ators through IValidation for content integrity 5. Content Registration: Use `IContentRe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~182-~182: Ensure spelling is correct
Context: ...erty interfaces enable composition over inheritance * Immutability: Record types encourage ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~183-~183: Ensure spelling is correct
Context: ...ge immutable data structures for thread safety * Separation of Concerns: Clear boundar...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~184-~184: Ensure spelling is correct
Context: ...between content models, validation, and registration * YAML-First: Built-in support for YAML...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~191-~191: Ensure spelling is correct
Context: ...terface implementation rather than deep inheritance 3. Validate Early: Implement `IValidatio...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~192-~192: Ensure spelling is correct
Context: ...nto catch content issues during build time 4. **Document YAML Attributes**: UseYamlMe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~193-~193: Ensure spelling is correct
Context: ...es with descriptions for clear metadata documentation 5. Respect Cancellation: Always honor `C...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~197-~197: Ensure spelling is correct
Context: ...te to services or handlers ## Internal Access The package declares internal visibility...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~207-~207: Ensure spelling is correct
Context: ...s maintain backward compatibility where possible * Unit tests cover new functionality ## L...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~216-~216: Ensure spelling is correct
Context: ...ementation consuming this extensibility package * NetEvolve.ForgingBlazor.Logging: Logg...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~217-~217: Ensure spelling is correct
Context: ...ogging extensions for the ForgingBlazor framework --- For questions, issues, or contributions,...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt
[grammar] ~13-~13: Ensure spelling is correct
Context: ... Show version information Commands: build Builds and generate...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt
[grammar] ~15-~15: Ensure spelling is correct
Context: ... Show version information Commands: build Builds and generate...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt
[grammar] ~13-~13: Ensure spelling is correct
Context: ... Show version information Commands: build Builds and generate...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt
[grammar] ~15-~15: Ensure spelling is correct
Context: ... Show version information Commands: build Builds and generate...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
src/NetEvolve.ForgingBlazor.Extensibility/README.md
223-223: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (3)
- GitHub Check: Build & Tests / Run Tests (ubuntu-latest) / Testing .NET solution
- GitHub Check: Build & Tests / Run Tests (windows-latest) / Testing .NET solution
- GitHub Check: Build & Tests / Run Tests (macos-latest) / Testing .NET solution
🔇 Additional comments (46)
src/NetEvolve.ForgingBlazor/Services/DefaultContentRegistration.cs (1)
17-17: Good design improvement.Sealing this internal implementation class is appropriate since it's not designed for inheritance and serves as a concrete default registration type.
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt (1)
8-8: The snapshot accurately reflects the actual CLI help output. All elements verified:
- Command description (line 8): "Creates a folder structure with example pages for a Forging Blazor application." matches implementation in CommandExample.cs
- Option flags (--project-path, --output-path) are correctly defined in CommandOptions.cs with matching descriptions
- LogLevel option appears in help output as a global option (added by CommandCli root command) with all severity levels and Information default correctly configured
No action required.
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySitemap.cs (2)
10-10: No action needed. The interface rename fromIPagePropertySitemaptoIPropertySitemaphas been completed throughout the codebase with no remaining stale references.
21-29: Clarify whether the 0-10 Priority range is intentional or should align with sitemap.xml standards.The
Priorityproperty usesint?with a documented range of 0-10, which differs from the standard sitemap.xml specification (0.0-1.0 decimal). If this higher-level API design is intentional for usability, clarify in the documentation that values are normalized to the standard range during sitemap generation. If unintentional, consider usingdouble?to align with the specification.src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyCategories.cs (1)
10-20: Rename is fine; double-check nullability expectations forCategories.
Categoriesis non-nullable but settable; ensure implementations always initialize it (e.g., to an empty set) to avoid nulls in practice.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt (1)
8-18: Snapshot update is coherent; watch for formatting flakiness (wrapping/spacing).
These help snapshots depend on alignment and line wrapping; please confirm tests run deterministically across target OS/CI environments (console width can change wrapping).tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt (1)
8-18: LGTM; same determinism concern as the DotNet9 snapshot.
If you’ve already stabilized help output formatting (fixed width / no wrapping), this is good.src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySeries.cs (1)
10-20: Rename looks consistent across the abstraction surface.
No functional concerns here.src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertySummary.cs (1)
1-19: Nice addition; interface + docs are clear and minimal.
Fits theIProperty*pattern well.src/NetEvolve.ForgingBlazor/Application.cs (1)
79-88: RunAsync flow is clean and appropriately fails fast on missing RootCommand.
TheGetService<RootCommand>() ?? throw ...pattern is good here.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt (1)
1-24: LGTM! CLI help text improvements are clear and consistent.The updated help output reflects the new global log-level option and improved command descriptions that explicitly mention "Forging Blazor application" for clarity.
Note: The static analysis grammar hint on line 15 is a false positive. "Show version information" is grammatically correct.
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt (1)
1-20: LGTM! Example command help text properly updated.The help output correctly reflects the updated command description and new long-form options including the global log-level option.
src/NetEvolve.ForgingBlazor.Extensibility/ServiceCollectionExtensions.cs (1)
1-76: LGTM! Well-implemented service transfer utility.The
TransferAllServicesmethod is properly designed with:
- Cached type references for performance optimization
- Comprehensive null-checking
- Clear filtering logic to exclude startup markers and service descriptor enumerables
- Thorough XML documentation explaining the purpose and behavior
The implementation correctly handles both
ServiceTypeandImplementationTypewhen filtering, ensuring startup-related services don't leak into child scopes.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt (1)
1-22: LGTM! Main help text properly reflects CLI improvements.The updated help output is consistent with the broader CLI enhancement effort, including the new global log-level option and more explicit command descriptions.
Note: The static analysis grammar hint on line 13 is a false positive. The text is grammatically correct.
src/NetEvolve.ForgingBlazor.Extensibility/NetEvolve.ForgingBlazor.Extensibility.csproj (1)
8-8: LGTM! System.CommandLine dependency supports new CLI infrastructure.The addition of
System.CommandLineis appropriate given the new command-line option handling introduced in this PR.tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandBuildTests.cs (1)
22-22: LGTM! Test simplification aligns with CLI standardization.The removal of the short
-oflag test case is consistent with the PR's move toward explicit long-form options (--output-path). The remaining test cases adequately cover the intended usage patterns.src/NetEvolve.ForgingBlazor/Services/ContentRegistration.cs (1)
17-17: LGTM! Sealing the class prevents unintended inheritance.Adding the
sealedmodifier is a good practice for internal implementation classes not designed for extension. This change improves the API surface clarity and aligns with similar modifications across the codebase.src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyPublishedOn.cs (1)
10-10: LGTM! Interface rename improves naming consistency.Renaming from
IPagePropertyPublishedOntoIPropertyPublishedOnis part of the broader effort to streamline property interface naming by removing the redundant "Page" prefix. This improves API clarity and aligns with similar renames across the extensibility layer.tests/NetEvolve.ForgingBlazor.Tests.Integration/Helper.cs (1)
7-7: LGTM! Builder rename applied correctly.The change from
ForgingBlazorApplicationBuilder.CreateDefaultBuildertoApplicationBuilder.CreateDefaultBuilderis consistent with the API refactoring across the codebase.tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs (3)
4-4: LGTM! Namespace import added for renamed types.The using directive correctly imports the namespace for the renamed
ApplicationBuilderandApplicationtypes.
12-12: LGTM! Builder rename applied correctly.The change from
ForgingBlazorApplicationBuilder.CreateDefaultBuildertoApplicationBuilder.CreateDefaultBuilderis consistent with the API refactoring.
15-15: LGTM! Proper async disposal pattern.Using
await usingensures theStringWriteris disposed asynchronously, which is the recommended pattern.src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs (3)
9-17: LGTM! Comprehensive class documentation.The XML documentation clearly describes the command's purpose and references related types.
20-22: LGTM! Field documentation added.The XML documentation clarifies the purpose of the service provider field.
46-51: LGTM! Method documentation added.The XML documentation properly documents the method parameters and return value.
src/NetEvolve.ForgingBlazor/Commands/CommandCli.cs (3)
32-32: LGTM! Improved command description.The updated description "Command-line interface for managing Forging Blazor applications." is more informative and professional than the previous "ForgingBlazor CLI".
43-43: LGTM! Correct API usage.Using
Add(command)instead ofSubcommands.Add(command)is the standard API for adding sub-commands to aRootCommand.
46-46: LGTM! Global log-level option added.Adding
CommandOptions.LogLevelto the root command makes logging control available globally across all sub-commands (since the option hasRecursive = trueper the CommandOptions definition).tests/NetEvolve.ForgingBlazor.Tests.Console/Program.cs (1)
10-10: LGTM! Builder rename applied correctly.The change from
ForgingBlazorApplicationBuilder.CreateDefaultBuildertoApplicationBuilder.CreateDefaultBuilderis consistent with the API refactoring across the codebase.src/NetEvolve.ForgingBlazor.Extensibility/README.md (1)
1-222: LGTM! Comprehensive and well-structured documentation.The README provides excellent coverage of the extensibility package, including:
- Clear overview of purpose and features
- Practical usage examples for common scenarios
- Architecture considerations and best practices
- Proper installation instructions and dependency information
The code examples are well-formatted and demonstrate proper usage patterns. This documentation will be valuable for developers extending the ForgingBlazor framework.
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IPropertyAuthor.cs (1)
9-9: Rename verified—all references to old interface name successfully updated.The rename from
IPagePropertyAuthortoIPropertyAuthoris a reasonable simplification that makes the interface name more concise while maintaining clarity. The change aligns with the broader PR objective of refactoring the API surface. Verification confirms no remaining references to the old interface name exist in the codebase.src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IContentRegister.cs (1)
14-14: LGTM! Modern marker interface syntax.The semicolon-terminated interface declaration is a cleaner way to define marker interfaces (introduced in C# 12). This syntax makes the intent explicit that the interface has no members.
src/NetEvolve.ForgingBlazor/Services/ContentRegister.cs (1)
1-13: LGTM!Clean rename to
ContentRegisterwith appropriate use of the semicolon-terminated class declaration for this marker service. The XML documentation clearly explains its purpose in the DI container.src/NetEvolve.ForgingBlazor.Extensibility/Models/BlogPostBase.cs (1)
14-24: Interface rename looks good for the first release.The simplified interface names (
IPropertyPublishedOn,IPropertyAuthor,IPropertyTags) are cleaner than the previousIPageProperty*variants. Since this is the first release of the Extensibility package, the breaking change is acceptable.The nullable properties are appropriate for optional blog metadata.
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt (1)
5-19: CLI help text improvements look good.The updated descriptions are clearer and more consistent with "Forging Blazor application" terminology. The new
-l, --log-levelglobal option with sensible default (Information) is a useful addition for debugging and troubleshooting.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt (1)
7-21: Consistent with other help snapshot.The help output for the
-hflag correctly mirrors the base help output with the updated descriptions and new log-level option.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt (1)
1-23: LGTM! Test snapshot accurately reflects updated CLI help text.The updated build command help output is clear and consistent, with improved option descriptions and the new log-level flag properly documented.
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt (1)
1-23: LGTM! Snapshot consistent across .NET versions.The DotNet10_0 snapshot correctly matches the DotNet9_0 version, ensuring consistent CLI help output across supported runtime versions.
src/NetEvolve.ForgingBlazor/Options/SegmentContentOptions.cs (1)
1-23: LGTM! Clean marker class for type-safe segment configuration.The empty sealed class provides type safety and allows future extensibility without breaking changes. The comprehensive documentation clearly explains its purpose and usage with AddSegment extension methods.
src/NetEvolve.ForgingBlazor/Options/ContentOptionsBase.cs (1)
26-52: LGTM! Well-designed base class for content configuration.The abstract base provides a clean foundation with appropriate defaults:
ContentTypecorrectly captures the generic type parameter for internal processingFileEncodingdefaults to UTF-8, which is the right choice for broad compatibility- The design allows derived classes to add type-specific configuration while sharing common options
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt (1)
1-23: LGTM! Serve command help text is clear and consistent.The updated help output for the serve command follows the same improved format as the build command, with clear descriptions and properly documented options.
src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/ISegmentBuilder.cs (1)
1-51: LGTM! Clean builder interface with fluent API design.The interface provides a well-structured builder pattern:
Servicesproperty enables segment-specific dependency injectionWithValidationmethod supports fluent chaining for adding multiple validation rules- Generic type constraint ensures type safety at compile time
- Documentation clearly guides proper usage through extension methods
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt (1)
1-23: LGTM! Consistent snapshot across .NET versions.The DotNet10_0 serve command snapshot correctly matches the DotNet9_0 version, maintaining consistency in CLI help output across supported runtimes.
src/NetEvolve.ForgingBlazor/Commands/CommandExample.cs (1)
6-10: Good: command now aligns with sharedProjectPath/OutputPathoptions.
This matches the centralization inNetEvolve.ForgingBlazor.Extensibility.Commands.CommandOptions.Also applies to: 40-44
src/NetEvolve.ForgingBlazor/ServiceCollectionExtensions.cs (2)
36-56: Core service registration + marker gate looks solid.
TheMarkerCoreServicesguard prevents duplicate registrations and keeps the DI setup predictable.
58-66: Good use ofIStartUpMarkerfor filtering startup-only services.
This aligns with the service-transfer filtering pattern used by commands.
| public interface IBlogSegmentBuilder<TBlogType> : ISegmentBuilder<TBlogType> | ||
| where TBlogType : BlogPostBase | ||
| { | ||
| /// <summary> | ||
| /// Adds a validation rule for blog posts in this segment. | ||
| /// </summary> | ||
| /// <param name="validation"> | ||
| /// The <see cref="IValidation{TBlogType}"/> instance that defines validation logic for blog posts. | ||
| /// Cannot be <see langword="null"/>. | ||
| /// </param> | ||
| /// <returns> | ||
| /// The same <see cref="IBlogSegmentBuilder{TBlogType}"/> instance for method chaining. | ||
| /// </returns> | ||
| /// <remarks> | ||
| /// This method signature hides the base interface method to return the blog-specific builder type, | ||
| /// enabling continued use of blog-specific configuration methods in a fluent chain. | ||
| /// </remarks> | ||
| new IBlogSegmentBuilder<TBlogType> WithValidation(IValidation<TBlogType> validation); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
rg -n -A 10 'AddBlogSegment' --type=cs | head -100Repository: dailydevops/forgingblazor
Length of output: 5827
Well-designed fluent builder interface, but implementation is incomplete.
The use of new to hide the base WithValidation method and return IBlogSegmentBuilder<TBlogType> is the correct pattern for maintaining fluent API chaining with derived builder types. The generic constraint on BlogPostBase ensures type safety.
However, the AddBlogSegment<TBlogType> implementation in ApplicationBuilderExtensions.cs (lines 69-79) is incomplete. The method validates its parameters correctly but then returns null! instead of instantiating and returning a BlogSegmentBuilder<TBlogType> instance. This will cause a NullReferenceException at runtime when developers attempt to use the builder. Complete the implementation by creating and returning a BlogSegmentBuilder<TBlogType> instance.
🤖 Prompt for AI Agents
In ApplicationBuilderExtensions.cs around lines 69 to 79 replace the placeholder
return null! with an actual BlogSegmentBuilder<TBlogType> instance: construct
and return a new BlogSegmentBuilder<TBlogType>(...) using the same validated
parameters/services (the IApplicationBuilder, the segment name/definition and
any service provider or options you validated earlier) so the method returns a
concrete builder instead of null; ensure you pass the required constructor
arguments in the correct order and include the proper namespace/import for
BlogSegmentBuilder.
| /// Implementations of this interface perform validation logic on pages and report issues through the provided <see cref="IValidationContext{TPageType}"/>. | ||
| /// </remarks> | ||
| public interface IValidation<in TPageType> | ||
| public interface IValidation<TPageType> | ||
| where TPageType : PageBase | ||
| { | ||
| /// <summary> | ||
| /// Validates the specified page and reports any errors or warnings to the validation context. | ||
| /// </summary> | ||
| /// <param name="page">The page instance to validate.</param> | ||
| /// <param name="context">The validation context used to record errors and warnings discovered during validation.</param> | ||
| void Validate(TPageType page, IValidationContext context); | ||
| void Validate(TPageType page, IValidationContext<TPageType> context); |
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.
Breaking API change: dropped variance + changed context type—ensure downstream implementations/callers are updated.
Removing in on IValidation<TPageType> and switching to IValidationContext<TPageType> will break existing validators and registrations; make sure all implementations and DI registrations compile and behave as intended with the new invariant generic context.
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidation.cs around
lines 10 to 20, the generic interface was changed to remove the contravariant
modifier and the context type was made invariant
(IValidationContext<TPageType>), which is a breaking API change for downstream
implementations and DI registrations; update every implementation and consumer
to use the new invariant signature (remove any relying on contravariance),
change method signatures and variables to accept IValidationContext<TPageType>
exactly, rebuild and fix compile errors, and verify all DI registrations
(service types and registration generics) match the new invariant interface so
validators are resolved at runtime.
| /// <summary> | ||
| /// Validates a property value against a condition and records an error if the condition is met. | ||
| /// </summary> | ||
| /// <typeparam name="TProperty">The type of the property being validated.</typeparam> | ||
| /// <param name="propertyValue"> | ||
| /// A function that extracts the property value from the page instance. | ||
| /// </param> | ||
| /// <param name="condition"> | ||
| /// A predicate function that returns <see langword="true"/> if the property value represents an error condition. | ||
| /// </param> | ||
| /// <param name="errorMessage"> | ||
| /// The error message to record if the condition evaluates to <see langword="true"/>. | ||
| /// </param> | ||
| /// <param name="propertyName"> | ||
| /// The name of the property being validated. This is automatically captured from the <paramref name="propertyValue"/> expression. | ||
| /// </param> | ||
| /// <returns> | ||
| /// The same <see cref="IValidationContext{TPageType}"/> instance for method chaining. | ||
| /// </returns> | ||
| /// <remarks> | ||
| /// This method uses caller argument expression to automatically capture the property name from the lambda expression, | ||
| /// providing clear diagnostic information without manual string literals. | ||
| /// </remarks> | ||
| IValidationContext<TPageType> ValidateError<TProperty>( | ||
| Func<TPageType, TProperty> propertyValue, | ||
| Func<TProperty, bool> condition, | ||
| string errorMessage, | ||
| [CallerArgumentExpression(nameof(propertyValue))] string? propertyName = null | ||
| ); |
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.
Docs likely over-promise: CallerArgumentExpression captures expression text, not a clean property name.
Either adjust the XML to say it captures the caller expression (recommended), or ensure implementations normalize it to a property path/name before emitting diagnostics.
Also applies to: 58-86
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor.Extensibility/Abstractions/IValidationContext.cs
around lines 28 to 56 (and similarly for the overloads at 58–86), the XML docs
claim CallerArgumentExpression will capture a "property name" but it actually
captures the caller expression text; either update the XML comments to state it
captures the caller expression text (e.g., "captures the caller expression text,
which may be a full expression, not a simple property name") or, if you want to
keep the "property name" wording, implement/describe a normalization step in the
interface contract and implementations that parses the expression text into a
clean property path/name before emitting diagnostics; choose one approach and
make consistent changes both to the summary/remarks and to any implementation
guidance in the interface comments for all affected overloads.
| <!-- Workaround: Remove before first release --> | ||
| <IsXampleProject>true</IsXampleProject> |
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.
🧩 Analysis chain
🏁 Script executed:
# Search for IsXampleProject in the codebase
rg "IsXampleProject" -nRepository: dailydevops/forgingblazor
Length of output: 429
🏁 Script executed:
# Read the full .csproj file
cat -n src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj
# Also search for "Workaround" comments across the repo
rg "Workaround" -nRepository: dailydevops/forgingblazor
Length of output: 1355
🏁 Script executed:
# Find all .csproj files
fd "\.csproj$" | head -20
# Search for IsXampleProject across all .csproj files
rg "IsXampleProject" --type csproj -B2 -A2Repository: dailydevops/forgingblazor
Length of output: 606
🏁 Script executed:
# Search for references to IsXampleProject in build files, props, targets, and config
rg "IsXampleProject" -t xml,yml,yaml,json,ps1,bat,sh -n
# Also check Directory.Build.props
find . -name "Directory.Build.props" -o -name "Directory.Build.targets" | xargs cat -n 2>/dev/nullRepository: dailydevops/forgingblazor
Length of output: 1283
🏁 Script executed:
# Check for any documentation mentioning IsXampleProject
rg -i "xample" -n | head -20
# Check parent directories for build configuration
cat -n Directory.Build.props 2>/dev/null || echo "No Directory.Build.props in root"
cat -n Directory.Build.targets 2>/dev/null || echo "No Directory.Build.targets in root"Repository: dailydevops/forgingblazor
Length of output: 2950
🏁 Script executed:
# Search all files for IsXampleProject references
rg "IsXampleProject" -n --no-heading
# Check if there are any GitHub workflows or CI/CD that might reference it
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "IsXampleProject" 2>/dev/null || echo "No CI/CD references found"
# Check README or other docs
find . -name "README*" -o -name "*.md" | head -5 | xargs grep -i "IsXampleProject" 2>/dev/null || echo "No documentation found"Repository: dailydevops/forgingblazor
Length of output: 478
🏁 Script executed:
# Check the Extensibility package .csproj
cat -n src/NetEvolve.ForgingBlazor.Extensibility/NetEvolve.ForgingBlazor.Extensibility.csproj
# Compare with Logging .csproj
cat -n src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csprojRepository: dailydevops/forgingblazor
Length of output: 1493
Resolve the timing conflict: "Remove before first release" comment contradicts releasing with this property present.
The IsXampleProject property in NetEvolve.ForgingBlazor.Logging.csproj is marked with a comment to remove it before the first release, yet this PR is releasing the Logging library for the first time. Additionally, IsXampleProject is not referenced anywhere in the visible build system, configuration, or documentation, making its purpose unclear.
Either remove this property and comment now, or clarify why the workaround must remain despite the first release.
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj
around lines 5-6, the IsXampleProject property and its "Remove before first
release" comment conflict with the fact this PR is the first release and the
property is unused; remove the IsXampleProject element and the comment now (or
if you discover any build/config references, instead update the comment to
explain why it must remain and add a TODO with a tracking issue number), then
run a quick build to confirm nothing breaks and commit the change.
| internal static IApplicationBuilder CreateEmptyBuilder(string[] args) => new ApplicationBuilder(args); | ||
|
|
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.
Public docs reference CreateEmptyBuilder, but the method is internal.
Either make it public (to match docs/examples and enable consumers) or update the XML docs to avoid pointing users at a non-public API.
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs around lines 257-258, the
method CreateEmptyBuilder is declared internal while public docs reference it;
change its accessibility to public so the API matches the documentation (i.e.,
make the method public), and ensure the XML documentation comments and any API
surface annotations reflect the new public visibility; alternatively, if you
want to keep it internal, update the XML docs and external documentation to stop
referencing CreateEmptyBuilder so consumers are not pointed to a non-public API.
| public static IBlogSegmentBuilder<TBlogType> AddBlogSegment<TBlogType>( | ||
| this IApplicationBuilder builder, | ||
| string segment | ||
| ) | ||
| where TBlogType : BlogPostBase | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
| ArgumentException.ThrowIfNullOrWhiteSpace(segment); | ||
|
|
||
| return null!; | ||
| } |
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.
Release-blocker: public extension methods return null!.
These will cause runtime NREs for any consumer calling AddBlogSegment / AddSegment. Return real builder instances (preferred) or throw NotImplementedException until implemented.
Concrete fix for blog segment (builder exists in this PR):
+using NetEvolve.ForgingBlazor.Builders;
...
public static IBlogSegmentBuilder<TBlogType> AddBlogSegment<TBlogType>(
this IApplicationBuilder builder,
string segment
)
where TBlogType : BlogPostBase
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentException.ThrowIfNullOrWhiteSpace(segment);
-
- return null!;
+ return new BlogSegmentBuilder<TBlogType>(builder, segment);
}For AddSegment<TPageType>, either implement an equivalent SegmentBuilder<TPageType> or fail fast:
-public static ISegmentBuilder<TPageType> AddSegment<TPageType>(this IApplicationBuilder builder, string segment)
- where TPageType : PageBase => null!;
+public static ISegmentBuilder<TPageType> AddSegment<TPageType>(this IApplicationBuilder builder, string segment)
+ where TPageType : PageBase =>
+ throw new NotImplementedException("Segment builder is not implemented yet.");Also applies to: 175-176
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor/ApplicationBuilderExtensions.cs around lines 69
to 79 (and similarly at 175-176), the public extension methods
AddBlogSegment/AddSegment currently return null! which will cause runtime NREs;
replace the null! with a real IBlogSegmentBuilder<TBlogType> (construct and
return the concrete BlogSegmentBuilder<TBlogType> that exists in this PR) or, if
not ready, throw a NotImplementedException to fail fast; ensure
ArgumentNull/ArgumentException checks remain and the returned builder is created
using the same IApplicationBuilder and segment parameters so consumers get a
valid instance instead of null.
| : base("serve", "Starts a development server for a Forging Blazor application.") | ||
| { | ||
| _serviceProvider = serviceProvider; | ||
|
|
||
| Add(Environment); | ||
| Add(IncludeDrafts); | ||
| Add(IncludeFuture); | ||
| Add(LogLevel); | ||
| Add(ProjectPath); | ||
| Add(OutputPath); | ||
|
|
||
| SetAction(ExecuteAsync); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
find . -type f -path "*Commands*" -name "*.cs"Repository: dailydevops/forgingblazor
Length of output: 574
🏁 Script executed:
git ls-files | grep -i serveRepository: dailydevops/forgingblazor
Length of output: 121
🏁 Script executed:
git ls-files | grep -i commandRepository: dailydevops/forgingblazor
Length of output: 2208
🏁 Script executed:
cat -n src/NetEvolve.ForgingBlazor/Commands/CommandServe.csRepository: dailydevops/forgingblazor
Length of output: 2857
🏁 Script executed:
grep -r "WebHost\|HttpListener\|KestrelServer\|DeveloperServer" src/ --include="*.cs" | head -10Repository: dailydevops/forgingblazor
Length of output: 51
🏁 Script executed:
grep -r "TODO\|FIXME\|WIP\|NotImplemented" src/NetEvolve.ForgingBlazor/Commands/ --include="*.cs"Repository: dailydevops/forgingblazor
Length of output: 51
🏁 Script executed:
grep -i "serve" tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/*/*.verified.txt | head -10Repository: dailydevops/forgingblazor
Length of output: 2005
serve command currently does nothing (and ignores options/cancellation).
The implementation accepts all options but returns success without executing any server logic. Users will see "Starts a development server" in help text but get a silent no-op. The parseResult and cancellationToken parameters are also unused.
Consider either implementing the actual server start or failing fast until implemented:
private Task<int> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken)
{
- _ = new ServiceCollection().TransferAllServices(_serviceProvider);
- return Task.FromResult(0);
+ cancellationToken.ThrowIfCancellationRequested();
+ throw new NotSupportedException("The 'serve' command is not implemented yet.");
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor/Commands/CommandServe.cs around lines 34 to 45,
the Serve command currently registers options but the ExecuteAsync action is a
no-op and ignores the parseResult and cancellationToken; either implement the
server startup flow or fail fast until implemented. Fix by wiring ExecuteAsync
to read the parsed options from parseResult (project/output paths,
includeDrafts/includeFuture flags), validate inputs, and then start the
development server logic (or call an existing server service from the service
provider), ensuring you observe the cancellationToken and return an appropriate
int exit code; if you cannot implement the server now, throw a
NotImplementedException or return a non-zero failure code with a clear error
message so the command does not silently succeed.
| <!-- Workaround: Remove before first release --> | ||
| <IsXampleProject>true</IsXampleProject> |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csprojRepository: dailydevops/forgingblazor
Length of output: 710
🏁 Script executed:
rg "IsXampleProject" --type csproj --type yml --type yamlRepository: dailydevops/forgingblazor
Length of output: 99
🏁 Script executed:
rg "IsXampleProject" -t cs -t powershell -t bashRepository: dailydevops/forgingblazor
Length of output: 103
🏁 Script executed:
rg "IsXampleProject" -g "!.git"Repository: dailydevops/forgingblazor
Length of output: 423
🏁 Script executed:
fd -e yml -e yaml -e json | head -20Repository: dailydevops/forgingblazor
Length of output: 131
🏁 Script executed:
find . -path "./.git" -prune -o -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.md" \) -print | grep -E "(workflow|github|pipeline|ci|release)" | head -10Repository: dailydevops/forgingblazor
Length of output: 462
🏁 Script executed:
cat GitVersion.ymlRepository: dailydevops/forgingblazor
Length of output: 468
🏁 Script executed:
rg "Target|Condition" --type xml | grep -i "xample\|IsXample" -A 2 -B 2Repository: dailydevops/forgingblazor
Length of output: 217
🏁 Script executed:
ast-grep --pattern 'Target {
$$$
Condition=$_
$$$
}'Repository: dailydevops/forgingblazor
Length of output: 51
🏁 Script executed:
rg "IsXampleProject" -A 3 -B 3Repository: dailydevops/forgingblazor
Length of output: 2438
Remove IsXampleProject property from three project files before merging.
The IsXampleProject property with the comment "Workaround: Remove before first release" appears in three .csproj files:
src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csprojsrc/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csprojtests/NetEvolve.ForgingBlazor.Tests.Console/NetEvolve.ForgingBlazor.Tests.Console.csproj
This property is unused and should be removed from all three files to clean up temporary workaround code before the first release.
🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csproj around lines 4-5
the temporary property IsXampleProject ("Workaround: Remove before first
release") should be removed; delete the <IsXampleProject> element from this file
and also remove the same property from the other two project files listed
(src/NetEvolve.ForgingBlazor.Logging/NetEvolve.ForgingBlazor.Logging.csproj and
tests/NetEvolve.ForgingBlazor.Tests.Console/NetEvolve.ForgingBlazor.Tests.Console.csproj)
so the unused workaround property is cleaned up across the repo before merging.
| namespace NetEvolve.ForgingBlazor.Options; | ||
|
|
||
| using NetEvolve.ForgingBlazor.Extensibility.Models; | ||
|
|
||
| /// <summary> | ||
| /// Provides configuration options for default content (pages) in a ForgingBlazor application. | ||
| /// </summary> | ||
| /// <typeparam name="TPageType"> | ||
| /// The page type that inherits from <see cref="PageBase"/>. | ||
| /// </typeparam> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// This sealed class represents the configuration for default content pages that don't belong to a specific segment or blog section. | ||
| /// It inherits all base configuration options from <see cref="ContentOptionsBase{TPageType}"/> without adding additional properties. | ||
| /// </para> | ||
| /// <para> | ||
| /// Use this options class when configuring default pages through the AddDefaultContent extension method. | ||
| /// </para> | ||
| /// </remarks> | ||
| /// <seealso cref="ContentOptionsBase{TPageType}"/> | ||
| /// <seealso cref="PageBase"/> | ||
| public sealed class DefaultContentOptions<TPageType> : ContentOptionsBase<TPageType> | ||
| where TPageType : PageBase; |
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.
This won’t compile: class declaration is missing a body ({ }).
Unless there’s a very custom source generator trick (unlikely), C# requires a class body.
public sealed class DefaultContentOptions<TPageType> : ContentOptionsBase<TPageType>
- where TPageType : PageBase;
+ where TPageType : PageBase
+{
+}🤖 Prompt for AI Agents
In src/NetEvolve.ForgingBlazor/Options/DefaultContentOptions.cs around lines 1
to 23, the class declaration ends with a semicolon and has no body which
prevents compilation; replace the semicolon with a proper class body by adding
opening and closing braces and, if no extra members are needed, leave the body
empty (i.e., declare public sealed class DefaultContentOptions<TPageType> :
ContentOptionsBase<TPageType> where TPageType : PageBase { } ), preserving the
generic constraint and inheritance.
| var args = Array.Empty<string>(); | ||
| var builder = ForgingBlazorApplicationBuilder.CreateDefaultBuilder(args); | ||
| var builder = ApplicationBuilder.CreateDefaultBuilder(args); | ||
| var app = builder.Build(); |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CreateEmptyBuilder usage and InternalsVisibleTo configuration
rg -n "CreateEmptyBuilder\s*\(" --type cs
rg -n "InternalsVisibleTo" --type csRepository: dailydevops/forgingblazor
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Read the test file to see the actual code at lines 8-10 and around line 21
cat -n tests/NetEvolve.ForgingBlazor.Tests.Unit/ApplicationBuilderTests.cs | head -30
# Search for InternalsVisibleTo in AssemblyInfo files
find . -name "AssemblyInfo.cs" -type f | xargs grep -l "InternalsVisibleTo" 2>/dev/null || echo "No AssemblyInfo.cs with InternalsVisibleTo found"
# Check for global usings or namespace level InternalsVisibleTo
rg -n "InternalsVisibleTo" --type cs --multiline || echo "No InternalsVisibleTo found"
# List project files to check for assembly attributes
find . -name "*.csproj" -o -name "GlobalAssemblyInfo.cs" | head -10Repository: dailydevops/forgingblazor
Length of output: 1638
🏁 Script executed:
#!/bin/bash
# Check CreateDefaultBuilder in ApplicationBuilder.cs
rg -n -B 2 "CreateDefaultBuilder\s*\(" src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs | head -20
# Check for InternalsVisibleTo in csproj files
rg -n "InternalsVisibleTo" src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csproj tests/NetEvolve.ForgingBlazor.Tests.Unit/NetEvolve.ForgingBlazor.Tests.Unit.csproj 2>/dev/null || echo "Checking csproj files..."
# View the relevant csproj sections
cat src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csproj | grep -A 5 -B 5 "InternalsVisibleTo" || echo "No InternalsVisibleTo in main csproj"
# Also check with head to see structure
head -50 src/NetEvolve.ForgingBlazor/NetEvolve.ForgingBlazor.csprojRepository: dailydevops/forgingblazor
Length of output: 1306
🏁 Script executed:
#!/bin/bash
# Search for CreateDefaultBuilder method definition (not just references)
rg -n "static.*CreateDefaultBuilder" src/NetEvolve.ForgingBlazor/ApplicationBuilder.cs
# Also look for all static methods in ApplicationBuilder
rg -n "^\s*(public|internal|private)?\s*static" src/NetEvolve.ForgingBlazor/ApplicationBuilder.csRepository: dailydevops/forgingblazor
Length of output: 750
Compile error: CreateEmptyBuilder is internal and test project has no InternalsVisibleTo access.
At lines 20-22, the test calls ApplicationBuilder.CreateEmptyBuilder(args) which is internal static. Since the test is in a separate assembly (NetEvolve.ForgingBlazor.Tests.Unit), this will fail to compile. Either add [assembly: InternalsVisibleTo("NetEvolve.ForgingBlazor.Tests.Unit")] to ApplicationBuilder.cs, make CreateEmptyBuilder public, or update the test to use the public CreateDefaultBuilder API instead.
(Note: Lines 8-10 use the public CreateDefaultBuilder method and are fine.)
🤖 Prompt for AI Agents
In tests/NetEvolve.ForgingBlazor.Tests.Unit/ApplicationBuilderTests.cs around
lines 8-10 (and note the failing call at lines 20-22), the test is calling the
internal CreateEmptyBuilder which is not visible to the test assembly; update
the test to use the public CreateDefaultBuilder API instead (replace
CreateEmptyBuilder(...) with CreateDefaultBuilder(...)), or if you must keep
CreateEmptyBuilder, expose it by either adding [assembly:
InternalsVisibleTo("NetEvolve.ForgingBlazor.Tests.Unit")] in
ApplicationBuilder.cs or changing CreateEmptyBuilder to public; prefer the first
option (update the test) for minimal surface-area change.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.