chore(review): trivial fixes from AI code-review pass#223
Merged
Conversation
Batch of low-risk fixes surfaced by the v0.3.3 AI code-review pass (#128). Larger findings are filed as separate Tier-2 sub-issues. Source — XML doc + small wording - Result.cs: class summaries reworded so they no longer claim the types are only produced by Try.Run (they're also used directly as repository / validation return types). - Result.cs: Failure() exception message now matches the protected ctor wording ("cannot be null, empty, or whitespace.") and the exception cref blocks point at <paramref name="errorMessage"/>. - Result.cs: <returns> added to Success() / Failure() factories; Result<T>.Success(value) now documents its <param> + <returns>; the private Result<T>.Value getter's <summary> mentions throw-on- failure inline (was only in <exception>), and the <exception> text is a complete sentence. - Try.cs: RunAsync<T> summary now says "asynchronously" (parity with the Action variant); Run<T> summary differentiates from Run by mentioning the return value. Tests — naming convention drift - ResultTests.cs: two tests whose names said "throw_Invalid- OperationException" actually assert ArgumentException; renamed to match observed behavior. - RunFuncTests.cs: 8 PascalCase test names migrated to the CLAUDE.md canonical snake_case Method_when_condition_expected_result pattern; the one named FunctionNullableInt_WithExceptionThrowingFunction_ReturnsDefault actually verifies Value-access throws on failed result and is renamed accordingly. csproj hygiene - src csproj: added <PackageTags> per Directory.Build.props's per-repo expectation; normalized mixed-tabs/spaces indent. - tests csproj: dropped dead netcoreapp3.1 <ItemGroup Condition> block (netcoreapp3.1 is not in <TargetFrameworks>, so the group never matched); dropped stale <Version>0.3.0</Version> (test project is IsPackable=false, the version field was unused). Examples - FSharp.DotNet8.Example/Program.fs: module name said FSharp.DotNet462.Example — copy-paste bug from the F#/.NET 4.6.2 sibling; renamed to FSharp.DotNet8.Example. Docs - docs/DOCFX-VERSION-PICKER.md: stripped a "no DateTime-Extensions paths leak" cross-repo reference that was carried over from the canonical fanout; replaced with generic wording. Out-of-scope follow-ups filed as Tier-2 sub-issues of #128: - xunit.runner.visualstudio 3.0.0 + xunit 2.9.3 mismatch on net8/9/10 (CLAUDE.md violation, behaviour change) - PublicApi/modern/PublicAPI.Shipped.txt missing Result<T>.Value.get -> T? entry (needs RS0016 sweep) - benchmarks csproj net8.0 → net10.0 (perf picture changes) - README database example uses sync IO inside Try.Run (opinion) - tests csproj/dir naming mismatch (Wolfgang.TryPattern.Tests.Unit vs Wolfgang.TryPattern.Tests) Closes #128
Chris-Wolfgang
added a commit
that referenced
this pull request
Jun 28, 2026
Tier-1 maintenance round — docs accuracy, code-review polish, and a single behaviour fix on the docs-site version picker. No public API or runtime behaviour change in Wolfgang.TryPattern itself, so a PATCH bump per SemVer. Contents this release: - Fix: docfx.json _appFooter version-picker bootstrap restored (caught during docs accuracy audit; fanned out to the 12 other affected Wolfgang.* repos in parallel) — PR #224 - Docs: README structure standardized to canonical fleet layout (License + Documentation hoisted; Code Quality section added) — PR #216, closes #135 #138 - Docs: accuracy audit fixes for CONTRIBUTING.md analyzer count, WORKFLOW_SECURITY.md actions/checkout version, RELEASE-WORKFLOW- SETUP.md jobs enumeration — PR #217, closes #139 - Chore: AI code-review pass — Result/Try XML doc improvements, test naming convention compliance, csproj hygiene, F# example module-name copy-paste fix; 5 larger findings filed as separate sub-issues — PR #223, closes #128 - Maintenance: verified and closed #127 (branch cleanup), #146 (PublicApiAnalyzer baseline), #152 (benchmarks TWAE) — all three already implemented at framework level by the canonical sync; this round was verification only.
7 tasks
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
1 participant
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
Batch-1 trivial / unambiguous fixes from the v0.3.3 Tier-1 AI code-review pass (#128). Larger findings filed as separate Tier-2 sub-issues; this PR holds only the safe, mechanical fixes.
Stacked on #217 (audit) → #216 (README) → vNext.
What's in this PR
Source XML doc / wording
Result.cs— class summaries for bothResultandResult<T>reworded so they don't claim the types are only produced byTry.Run(they're also used directly as repository / validation return types — see README Real-World Examples)Result.cs—Failure()exception message now matches the protected ctor wording ("errorMessage cannot be null, empty, or whitespace.")Result.cs—<exception>blocks reference<paramref name="errorMessage"/>for clickabilityResult.cs—<returns>added toSuccess()/Failure()factories;Result<T>.Success(value)documents its<param>+<returns>Result.cs—Result<T>.Valuesummary now mentions throw-on-failure inline (was only in<exception>);<exception>text is a complete sentenceTry.cs—RunAsync<T>summary says "asynchronously" (parity with the Action variant);Run<T>summary differentiates fromRunby mentioning the return valueTests — naming convention drift
ResultTests.cs— 2 tests whose names saidthrow_InvalidOperationExceptionactually assertArgumentException; renamed to match observed behaviorRunFuncTests.cs— 8 PascalCase test names migrated to the CLAUDE.md canonicalMethod_when_condition_expected_resultsnake_case pattern; the misleadingFunctionNullableInt_WithExceptionThrowingFunction_ReturnsDefault(actually tests Value-access throws) renamed accordinglycsproj hygiene
<PackageTags>perDirectory.Build.props's per-repo expectation; normalized mixed-tabs/spaces indentnetcoreapp3.1 <ItemGroup Condition>(TFM not in<TargetFrameworks>so never matched); dropped stale<Version>0.3.0</Version>(project isIsPackable=false, version was unused)Examples
FSharp.DotNet8.Example/Program.fs— module name wasFSharp.DotNet462.Example(copy-paste bug); renamedDocs
docs/DOCFX-VERSION-PICKER.md— stripped a "no DateTime-Extensions paths leak" cross-repo reference left over from the canonical fanoutOut-of-scope follow-ups (filed as sub-issues)
Result<T>.Value.get -> T?Try.RunWolfgang.TryPattern.Tests.UnitvsWolfgang.TryPattern.Tests)These were too risky / opinion-dependent / scope-breaking to bundle here. They'll be worked separately so any regression is easy to bisect.
Test plan
dotnet build -c Releasegreen (0 errors, only NuGet TFM-support warnings on test net5/6/7 — unrelated)dotnet test -c Release -f net10.0— 88/88 passing[Fact]/[Theory]attributes, not naming)Closes #128
Part of the v0.3.3 Tier-1 maintenance pilot (vNext: #121).