refactor: convention cleanup (comments, sealing, CancellationToken) - #139
Merged
Conversation
Fixes the QUICK-FIX findings from the shared-conventions audit: strips narrative/dead comments and internal-only XML docs, fixes a malformed stacked XML doc in Initialization.cs, seals MailService types and ~28 test classes with no subclassers, converts DataProtectionKeyDocument to a sealed record, threads CancellationToken through SendEmailMessageHandler.Handle, drops two redundant StateHasChanged() calls in Setup.razor, adds missing Arrange/Act/Assert comments to 9 tests, and replaces default(TValue) with bare default in ErrorComponentContext.
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Initialization.cs, a handful of Razor pages), and stripped XML doc comments from internal/private members, keeping XML docs only on genuinely public API surface.Initialization.cs: two stacked<summary>blocks were both attached toEnsureKeyRingProtected, leavingBuildForwardedHeadersOptionsundocumented. Reattached (and, per the internal-doc-stripping rule, compressed to a one-line comment).default(TValue)with baredefaultinErrorComponentContext.cs; the direct replacement was ambiguous betweenValueTask<T>'s two constructors, so it uses a namedresult:argument to disambiguate.SendEmailMessageHandler,MailConfiguration,MailMessage, and 28 test classes with no subclassers (verified via repo-wide grep before sealing each). LeftSmtpEmailSenderandLocalizedComponentBaseunsealed as intended.DataProtectionKeyDocumentto asealed record; the Marten document alias is set explicitly (DocumentAlias("dataprotectionkeydocument")), so the storage contract is unaffected either way.CancellationTokenthroughSendEmailMessageHandler.HandleintoIEmailSender.SendAsync, and updated its tests accordingly.StateHasChanged()calls inSetup.razor'sOnValidSubmithandlers, matching the pattern already used inEmailSettingsPage.razor.// Arrange/// Act/// Assertcomments to the 9 flagged tests.AppPageTitle investigation (item 8)
The audit's concern — that
AppPageTitlefromAndreGoepel.Design.Blazormight never actually render<PageTitle>/set the browser tab title — does not hold up. The component's source (both ondesign-blazor'smainand at thev1.4.2tag pinned in this repo'sDirectory.Packages.props) contains<PageTitle>@DocumentTitle</PageTitle>, the framework's own component. The compiledAndreGoepel.Design.Blazor.dll(v1.4.2, from the local NuGet cache) does contain aPageTitlestring reference in its metadata, confirming the Razor compiler emitted a reference to the framework type — the audit's binary inspection apparently missed it or checked the wrong artifact.HeadOutlet/HeadContent/document.titleare correctly absent from this library's DLL: that's a host-app concern (wired inApp.razor's<HeadOutlet />), not something a Razor class library needs to reference, and the sample host app has it configured correctly. No code changes were made indesign-blazor— that stays out of scope for this task, but the human maintainer can now close out that audit finding as a false positive.Test plan
dotnet csharpier format .dotnet build— succeeds, no new warnings/errors (2 pre-existing nullable warnings in test files untouched by this change)dotnet test --filter "FullyQualifiedName!~E2ETests"— 29 + 84 passed, 0 faileddotnet list package --vulnerable --include-transitive— no vulnerable packagesCloses #137
🤖 Generated with Claude Code