docs(backlog): register 23 historical architectural decisions#336
Merged
piotrzajac merged 4 commits intomasterfrom Apr 22, 2026
Merged
docs(backlog): register 23 historical architectural decisions#336piotrzajac merged 4 commits intomasterfrom
piotrzajac merged 4 commits intomasterfrom
Conversation
Mines the full git history to reconstruct 22 architectural decisions made from 2017 to 2026 and assigns each an accurate date from the originating commit. Moves the pre-existing performance benchmark decision to decision-23. Also adds task-19 (Add performance benchmarks). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 10 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughThis pull request adds 23 new architectural decision records and 1 backlog task document to formalize design choices for an AutoFixture XUnit2 mocking library project. It also updates the backlog configuration with a "performance" label and applies a minor spelling correction to a completed task documentation file. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
.backlog/decisions/decision-11 - Exclude-Moq-from-Dependabot-group-remaining-NuGet-updates.md (1)
22-24: Add an explicit Moq re-evaluation cadence.“Frozen until explicitly upgraded” is clear, but it would be safer to add a periodic review trigger (for example, quarterly/manual security review) to avoid long-term drift.
Suggested wording
## Consequences - Moq version is frozen until explicitly and consciously upgraded. +- Moq freeze is re-evaluated on a regular cadence (e.g., quarterly) and on any security advisory. - Grouped updates reduce weekly PR noise from many individual bumps to a handful of grouped PRs. - Any future Moq upgrade must be reviewed for licensing, telemetry, and breaking changes before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.backlog/decisions/decision-11 - Exclude-Moq-from-Dependabot-group-remaining-NuGet-updates.md around lines 22 - 24, Add a periodic re-evaluation cadence for Moq by updating the decision text that currently says "Moq version is frozen until explicitly and consciously upgraded." — append a sentence such as "Moq will be re-evaluated on a quarterly basis (or upon major security advisories) for licensing, telemetry, security, and breaking changes" immediately after that bullet so future reviewers have a clear scheduled trigger for review; ensure the new sentence references the same review criteria already listed (licensing, telemetry, breaking changes) and matches the tone/format of the existing bullets..backlog/decisions/decision-23 - Performance-benchmark-tooling-and-approach.md (1)
69-71: Clarify benchmark design vs. IDE behavior.The statement "Each benchmark iteration therefore calls
GetData()twice to reflect this reality" conflates IDE discovery behavior with benchmark design. It's unclear whether:
- The benchmarks are intentionally designed to call
GetData()twice per iteration to simulate IDE behavior, or- This is merely describing IDE behavior without prescribing benchmark design
Benchmarks should measure the library's performance in isolation, not replicate IDE-specific quirks. If the intent is to simulate real-world IDE usage, state this explicitly and justify why IDE behavior should influence benchmark design.
📋 Suggested clarification
-The double-call concern is real at the **IDE layer**: Visual Studio Test Explorer and Rider -each trigger an independent discovery pass on project load and again on "Run". Each benchmark -iteration therefore calls `GetData()` **twice** to reflect this reality. +The double-call concern is real at the **IDE layer**: Visual Studio Test Explorer and Rider +each trigger an independent discovery pass on project load and again on "Run". Benchmarks +may optionally call `GetData()` twice per iteration to simulate this IDE behavior, though +single-call benchmarks remain valid for measuring core library performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.backlog/decisions/decision-23 - Performance-benchmark-tooling-and-approach.md around lines 69 - 71, Clarify whether the double-call to GetData() is part of the benchmark design or merely an observation of IDE behavior: in the paragraph mentioning "Each benchmark iteration therefore calls `GetData()` **twice**" explicitly state intent (e.g., "Benchmarks intentionally call `GetData()` twice per iteration to simulate IDE discovery behavior") and add a sentence justifying that choice (or remove the sentence if benchmarks should measure the library in isolation); separate the IDE layer description (Visual Studio Test Explorer and Rider discovery behavior) from the benchmark methodology and reference the `GetData()` call to make the distinction unambiguous..backlog/decisions/decision-4 - Publish-Core-bundled-into-module-packages-not-standalone.md (1)
23-23: Clarify the distinction between packaging strategy and API visibility.The statement "its public API is effectively internal" is misleading. Core's public types remain public in the .NET type system and are accessible to consumers of the module packages. The bundling strategy controls distribution, not visibility. Consider rephrasing to: "Core is an implementation detail of the distribution strategy; consumers interact with it through the module packages."
📝 Suggested rephrasing
-- Core is an implementation detail; its public API is effectively internal to the three published packages. +- Core is an implementation detail of the distribution strategy; consumers interact with it indirectly through the three published module packages, though its public types remain accessible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.backlog/decisions/decision-4 - Publish-Core-bundled-into-module-packages-not-standalone.md at line 23, Replace the misleading sentence "Core is an implementation detail; its public API is effectively internal to the three published packages." with a clearer distinction between packaging and type visibility: rephrase to something like "Core is an implementation detail of the distribution strategy; consumers interact with it through the module packages." Ensure the wording clarifies that Core's public types remain public in the .NET type system and that bundling affects distribution, not API visibility..backlog/decisions/decision-22 - Enforce-Conventional-Commits.md (1)
11-16: Consider documenting merge commit exclusion for completeness.Based on learnings, merge commit messages are intentionally excluded from Conventional Commits linting (via
--no-mergesflag ingit rev-list). Adding this detail to the decision would provide a more complete picture of the enforcement strategy and prevent future confusion.📋 Suggested addition for merge commit handling
Enforce Conventional Commits format (`<type>(<scope>): <description>`) on every commit, +where scope is optional. Merge commits are intentionally excluded from linting. Enforcement uses a Husky.NET `commit-msg` hook for local validation and CommitLint.Net in CI to block merges that contain non-conforming commit messages.Based on learnings: merge commits are intentionally excluded from linting in both PR and push-to-master scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.backlog/decisions/decision-22 - Enforce-Conventional-Commits.md around lines 11 - 16, Update the decision to explicitly state that merge commits are excluded from Conventional Commits linting: add a sentence under "Decision" noting that linting uses git rev-list --no-merges (or equivalent) so merge commits are skipped, and clarify this applies to both the Husky.NET commit-msg local validation and the CommitLint.Net CI enforcement to avoid future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.backlog/decisions/decision-19 - Add-CustomizeWith-parameter-attributes.md:
- Line 27: The document decision-19 currently references "the other
data-narrowing parameter attributes in the same parameter list" which
forward-references features introduced in decision-18 (dated later); to fix,
either update decision-19's date to be after decision-18 or remove the forward
reference phrase from the sentence on line 27 (the fragment "and the other
data-narrowing parameter attributes in the same parameter list"); locate the
sentence in decision-19 and edit it to either adjust the date metadata or remove
that clause so the decision does not reference future features.
In @.backlog/decisions/decision-22 - Enforce-Conventional-Commits.md:
- Line 13: Update the commit format line to use placeholder angle-bracket
notation (e.g., <type>(<scope>): <description>) instead of square brackets, and
add a brief note that scope is optional in this project (include a sentence like
"scope is optional; this project sets conventional-commit.scopes.enabled to
false"). Reference the decision title "decision-22 -
Enforce-Conventional-Commits" and the configuration key
conventional-commit.scopes.enabled to ensure the reader understands the
intentional optionality.
- Line 4: Update the decision entry's date field to match its originating commit
by changing the value of the date: property in the "decision-22 -
Enforce-Conventional-Commits.md" file from '2026-04-22' to '2026-04-10'; locate
the line containing the date: token and replace the timestamp string accordingly
so the file reflects the commit date.
In @.backlog/decisions/decision-3 -
Support-multiple-mocking-frameworks-as-separate-modules.md:
- Around line 17-18: The sentence claiming the attributes (`[AutoMockData]`,
`[InlineAutoMockData]`, `[MemberAutoMockData]`) are "prefixed with the framework
name" is incorrect; update the wording to state that each module (e.g., the
AutoMoq, AutoNSubstitute, and AutoFakeItEasy modules) exposes attributes with
the same public names/shape (e.g., AutoMockDataAttribute,
InlineAutoMockDataAttribute, MemberAutoMockDataAttribute) but differing
implementations via their Customize(IFixture) override, to avoid implying
different API names per framework.
In @.backlog/tasks/task-19 - Add-performance-benchmarks.md:
- Line 19: The link label and target disagree: the visible text shows
"DECISION-1" while the URL points to "decision-23 — Performance benchmark
tooling and approach"; update the markdown so the visible label and the link
target match (either change the label to "DECISION-23 — Performance benchmark
tooling and approach" or change the URL to point to the correct decision
referenced by "DECISION-1"), ensuring the link text and href are consistent in
the line containing "recorded in [DECISION-1 — Performance benchmark tooling and
approach](../decisions/decision-23%20-%20Performance-benchmark-tooling-and-approach.md)".
---
Nitpick comments:
In @.backlog/decisions/decision-11 -
Exclude-Moq-from-Dependabot-group-remaining-NuGet-updates.md:
- Around line 22-24: Add a periodic re-evaluation cadence for Moq by updating
the decision text that currently says "Moq version is frozen until explicitly
and consciously upgraded." — append a sentence such as "Moq will be re-evaluated
on a quarterly basis (or upon major security advisories) for licensing,
telemetry, security, and breaking changes" immediately after that bullet so
future reviewers have a clear scheduled trigger for review; ensure the new
sentence references the same review criteria already listed (licensing,
telemetry, breaking changes) and matches the tone/format of the existing
bullets.
In @.backlog/decisions/decision-22 - Enforce-Conventional-Commits.md:
- Around line 11-16: Update the decision to explicitly state that merge commits
are excluded from Conventional Commits linting: add a sentence under "Decision"
noting that linting uses git rev-list --no-merges (or equivalent) so merge
commits are skipped, and clarify this applies to both the Husky.NET commit-msg
local validation and the CommitLint.Net CI enforcement to avoid future
confusion.
In @.backlog/decisions/decision-23 -
Performance-benchmark-tooling-and-approach.md:
- Around line 69-71: Clarify whether the double-call to GetData() is part of the
benchmark design or merely an observation of IDE behavior: in the paragraph
mentioning "Each benchmark iteration therefore calls `GetData()` **twice**"
explicitly state intent (e.g., "Benchmarks intentionally call `GetData()` twice
per iteration to simulate IDE discovery behavior") and add a sentence justifying
that choice (or remove the sentence if benchmarks should measure the library in
isolation); separate the IDE layer description (Visual Studio Test Explorer and
Rider discovery behavior) from the benchmark methodology and reference the
`GetData()` call to make the distinction unambiguous.
In @.backlog/decisions/decision-4 -
Publish-Core-bundled-into-module-packages-not-standalone.md:
- Line 23: Replace the misleading sentence "Core is an implementation detail;
its public API is effectively internal to the three published packages." with a
clearer distinction between packaging and type visibility: rephrase to something
like "Core is an implementation detail of the distribution strategy; consumers
interact with it through the module packages." Ensure the wording clarifies that
Core's public types remain public in the .NET type system and that bundling
affects distribution, not API visibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a28db82-aacb-49a7-a50a-2530073d2114
📒 Files selected for processing (26)
.backlog/completed/task-12 - Eliminate-intermediate-array-allocations.md.backlog/config.yml.backlog/decisions/decision-1 - Decouple-xUnit-from-AutoFixture-via-provider-pattern.md.backlog/decisions/decision-10 - Adopt-GitHub-Actions-as-CI-CD-platform.md.backlog/decisions/decision-11 - Exclude-Moq-from-Dependabot-group-remaining-NuGet-updates.md.backlog/decisions/decision-12 - Integrate-Stryker-mutation-testing-into-CI.md.backlog/decisions/decision-13 - Enable-CodeQL-for-C-static-security-analysis.md.backlog/decisions/decision-14 - Adopt-CodeRabbit-for-AI-powered-PR-review.md.backlog/decisions/decision-15 - Publish-symbols-and-enable-SourceLink.md.backlog/decisions/decision-16 - Integrate-FOSSA-for-license-compliance-scanning.md.backlog/decisions/decision-17 - Integrate-Semgrep-for-SAST-scanning.md.backlog/decisions/decision-18 - Add-data-narrowing-parameter-attributes.md.backlog/decisions/decision-19 - Add-CustomizeWith-parameter-attributes.md.backlog/decisions/decision-2 - Extract-shared-logic-into-a-Core-assembly.md.backlog/decisions/decision-20 - Adopt-Qodana-for-code-quality-scanning.md.backlog/decisions/decision-21 - Integrate-Snyk-for-dependency-vulnerability-scanning.md.backlog/decisions/decision-22 - Enforce-Conventional-Commits.md.backlog/decisions/decision-23 - Performance-benchmark-tooling-and-approach.md.backlog/decisions/decision-3 - Support-multiple-mocking-frameworks-as-separate-modules.md.backlog/decisions/decision-4 - Publish-Core-bundled-into-module-packages-not-standalone.md.backlog/decisions/decision-5 - Target-netstandard2.0-2.1-and-net472-net48.md.backlog/decisions/decision-6 - Delay-sign-assemblies-full-signing-in-CI-only.md.backlog/decisions/decision-7 - Suppress-virtual-member-population-via-customization.md.backlog/decisions/decision-8 - Share-fixture-across-member-data-rows-by-default.md.backlog/decisions/decision-9 - Use-GitVersion-in-ContinuousDelivery-mode.md.backlog/tasks/task-19 - Add-performance-benchmarks.md
Renumbers all 23 decisions so their IDs match the chronological order implied by the date field. Decision-23 (Performance benchmark) remains unchanged as the most recent entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Test plan
.backlog/decisions/contains exactly 23 files (decision-1throughdecision-23)datefield inyyyy-mm-ddformat matching its historical origindecision-23is the performance benchmark decisiontask-19referencesdecision-23🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Documentation
Chores