docs(adr): add Architecture Decision Records (#372)#442
Merged
nanotaboada merged 1 commit intomasterfrom Apr 2, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
✅ Files skipped from review due to trivial changes (16)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an ADR system: new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 (1)
adr/0009-implement-in-memory-caching.md (1)
19-19: Include sliding expiration in the decision text for accuracy.The implementation in
src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.csuses both a 1-hour absolute expiration and a 10-minute sliding expiration. Consider documenting both so the ADR fully reflects runtime behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@adr/0009-implement-in-memory-caching.md` at line 19, Update the ADR text to mention that the in-memory cache uses both a one-hour absolute expiration and a 10-minute sliding expiration (matching the implementation in PlayerService and IMemoryCache usage), so state that cached entries expire after 1 hour absolute but their TTL is extended by 10 minutes on access; ensure wording references IMemoryCache and PlayerService to link the decision to the runtime behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@adr/0003-use-sqlite-for-data-storage.md`:
- Line 17: The ADR uses a placeholder path for the SQLite file; update the
sentence in adr/0003-use-sqlite-for-data-storage.md (the line referencing the DB
file) to use the actual project path and filename used elsewhere
(storage/players-sqlite3.db) so it matches the repository docs and Docker volume
instructions.
In `@adr/0004-use-uuid-as-database-primary-key.md`:
- Line 17: The ADR incorrectly claims IDs are generated in application code via
Guid.NewGuid(); instead update the ADR to state that ID generation is delegated
to EF Core/database (reference entity.Property(...).ValueGeneratedOnAdd() in
PlayerDbContext and the Player.Id field initializer public Guid Id { get; set; }
= Guid.NewGuid() being superseded), and revise the positive consequences
paragraph to reflect that IDs are DB/ORM-generated; if you intended
application-side generation instead, remove ValueGeneratedOnAdd() and ensure Id
is assigned (e.g., via constructor or field initializer) before SaveChanges so
the ADR matches the chosen implementation.
In `@adr/0005-use-squad-number-as-api-mutation-key.md`:
- Line 19: The ADR text contradicts itself about using UUIDs in URLs; update the
wording at the sentence referencing Id to scope the decision to mutation and
primary lookup routes only — state that squadNumber will be used as the route
parameter for single-resource mutation endpoints (PUT /players/squadNumber/{n}
and DELETE /players/squadNumber/{n}) and for primary lookup routes where
applicable, and remove or change the documented GET /players/{id:Guid} example
so it uses squadNumber (GET /players/squadNumber/{n}) or otherwise explicitly
call out when UUID-based GETs are allowed; adjust the lines referencing GET
/players/{id:Guid} to match this scoped decision.
In `@adr/0006-use-rfc-7807-problem-details-for-errors.md`:
- Line 32: Update the sentence about the `type` field to state that while
RFC7807's canonical URI (https://tools.ietf.org/html/rfc7807) is commonly used
as a default, this project does not rely on a single global URI — individual
controllers may set custom problem `Type` URIs (see the PlayerController.cs
usage of the Type property for 409 responses). Rephrase to indicate a general
default exists but controllers like PlayerController may override the `Type`
with project- or problem-specific URIs.
In `@adr/0008-use-automapper-for-dto-mapping.md`:
- Line 17: The ADR misstates how the Player.Id is protected: instead of saying
the Id is "explicitly ignored in the PlayerRequestModel → Player mapping,"
update the text to state that PlayerRequestModel simply does not expose an Id
property so clients cannot set it (i.e., the absence of the Id on
PlayerRequestModel enforces protection), and remove the claim that this is
handled by "a single declarative line in the profile"; retain references to
PlayerMappingProfile and AddAutoMapper(typeof(PlayerMappingProfile)) so readers
know where mappings live and clarify that no explicit .ForMember(...Ignore()) is
required for Id because the request DTO lacks that property.
---
Nitpick comments:
In `@adr/0009-implement-in-memory-caching.md`:
- Line 19: Update the ADR text to mention that the in-memory cache uses both a
one-hour absolute expiration and a 10-minute sliding expiration (matching the
implementation in PlayerService and IMemoryCache usage), so state that cached
entries expire after 1 hour absolute but their TTL is extended by 10 minutes on
access; ensure wording references IMemoryCache and PlayerService to link the
decision to the runtime behavior.
🪄 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: bd405eeb-2a5f-4d28-be74-51d6f9c81789
📒 Files selected for processing (17)
.github/copilot-instructions.mdCHANGELOG.mdCONTRIBUTING.mdREADME.mdadr/0001-adopt-traditional-layered-architecture.mdadr/0002-use-mvc-controllers-over-minimal-api.mdadr/0003-use-sqlite-for-data-storage.mdadr/0004-use-uuid-as-database-primary-key.mdadr/0005-use-squad-number-as-api-mutation-key.mdadr/0006-use-rfc-7807-problem-details-for-errors.mdadr/0007-use-fluentvalidation-over-data-annotations.mdadr/0008-use-automapper-for-dto-mapping.mdadr/0009-implement-in-memory-caching.mdadr/0010-use-serilog-for-structured-logging.mdadr/0011-use-docker-for-containerization.mdadr/0012-use-stadium-themed-semantic-versioning.mdadr/README.md
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
nanotaboada
force-pushed
the
docs/add-architecture-decision-records
branch
from
b6d6fd6 to
256c00e
Compare
|
This was referenced Apr 2, 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
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
adr/directory with 12 Architecture Decision Records (ADRs) documenting the key architectural and technology decisions in this projectadr/README.mdwith ADR index, template, and guidance on when to create new ADRsREADME.mdwith an Architecture Decisions section and ToC entryCONTRIBUTING.mdwith ADR guidance for contributors.github/copilot-instructions.mdwith ADR context loading instructions for AI agentsADRs included
Test plan
dotnet build --configuration Release— passed (0 warnings, 0 errors)dotnet test --settings .runsettings— passed (39/39)dotnet csharpier --check .— passedCloses #372
🤖 Generated with Claude Code
Summary by CodeRabbit