-
Notifications
You must be signed in to change notification settings - Fork 14
chore: add GitHub Copilot instructions file #278
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
Add comprehensive .github/copilot-instructions.md with detailed guidance for AI assistance including: - Project overview and architectural patterns - Coding conventions and technology stack details - Common patterns for ASP.NET Core development - Domain knowledge with football position abbreviations - Error handling patterns and data flow examples - Clear guidelines for Copilot focus areas Closes #271
WalkthroughAdds a new repository-level guidance document at .github/copilot-instructions.md outlining tech stack, architecture, conventions, testing, and explicit instructions for GitHub Copilot usage and avoidance. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
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: 0
🧹 Nitpick comments (11)
.github/copilot-instructions.md (11)
10-10: Use “an SQLite” (pronunciation fix).“SQLite” starts with a vowel sound (“ess…”). Minor readability improvement.
-- **EF Core** with a **SQLite** database for simplicity +- **EF Core** with an **SQLite** database for simplicity
233-233: Remove trailing colons in headings to satisfy markdownlint (MD026).Current CI linters will flag these.
-### Choose EF Core When: +### Choose EF Core When -### Consider Raw SQL/Dapper When: +### Consider Raw SQL/Dapper WhenAlso applies to: 239-239
64-64: Add 204 No Content to the status code examples.Common for successful DELETE/PUT-without-body responses; helpful for Copilot completions.
-- Using appropriate HTTP status codes (200, 201, 400, 404, 409, etc.) +- Using appropriate HTTP status codes (200, 201, 204, 400, 404, 409, etc.)
247-249: Clarify API design guidance to include 204.Aligns with REST practice in this codebase.
-- Return appropriate status codes (200, 201, 400, 404, 409, 500) +- Return appropriate status codes (200, 201, 204, 400, 404, 409, 500)
92-92: Avoid asserting “already configured” for DbContext pooling.If the project switches providers/environments, this may drift. Prefer guidance wording.
-- Use `DbContextPool` for better performance (already configured) +- Prefer `AddDbContextPool` for better performance when appropriateIf desired, I can scan Program.cs to confirm whether pooling is currently enabled and adjust wording accordingly.
210-215: Controller snippet should use ControllerBase helpers, not TypedResults.The doc frames controllers (not minimal APIs). Use
Conflict(...)for clarity.- return TypedResults.Conflict($"Squad number {squadNumber} already exists"); + return Conflict($"Squad number {squadNumber} already exists");
87-96: Call out CancellationToken propagation.Helps Copilot suggest cancellable async code across layers and EF Core.
- Follow async/await patterns consistently +- Propagate CancellationToken through controller ➜ service ➜ repository ➜ EF Core calls
130-135: Soften Serilog wording to avoid mismatch with actual setup.Use conditional phrasing; keep
ILogger<T>as the abstraction.-### Logging with Serilog +### Logging - **Structured Logging**: Consistent log message templates - **Log Levels**: Appropriate use of Information, Warning, Error - **Context**: Include relevant data in log messages -- **Configuration**: File and console sinks configured +- **Configuration**: Use `ILogger<T>`; Serilog sinks (file/console) if configured
69-77: Explicitly discourage blocking async (.Result/Wait).Prevents deadlocks/thread-pool starvation in ASP.NET Core.
- Using `ConfigureAwait(false)` in ASP.NET Core contexts +- Blocking on async calls (e.g., `.Result`, `.Wait()`); always use async/await end-to-end
99-104: Add guidance on secret management.Complements appsettings guidance and Docker usage.
- .NET 8 SDK is required - All configurations live in `appsettings.*.json` files +- Never commit secrets; use User Secrets in dev and environment variables/KeyVault in containers
80-86: Suggest adding integration testing guidance.Improves Copilot’s test scaffolds beyond unit tests.
- Use **Moq** for mocking - Prefer testing **service logic** and **controller behavior** - Place unit tests under `test/` following structure already present (e.g., `Unit/PlayerServiceTests.cs`) - **Test Data**: Use faker patterns for consistent test data generation - **Assertions**: FluentAssertions for readable test assertions +- Consider integration tests with `WebApplicationFactory<TEntryPoint>` for API endpoints
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/copilot-instructions.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.github/copilot-instructions.md (2)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20250414191223_InitialCreate.cs (1)
Dotnet.Samples.AspNetCore.WebApi.Migrations(6-49)src/Dotnet.Samples.AspNetCore.WebApi/Migrations/PlayerDbContextModelSnapshot.cs (1)
Dotnet.Samples.AspNetCore.WebApi.Migrations(10-66)
🪛 LanguageTool
.github/copilot-instructions.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...this repository. ## 🎯 Project Overview This project is a proof-of-concept Web A...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ... a proof-of-concept Web API built using: - .NET 8 (LTS) - ASP.NET Core - **EF...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...pt Web API built using: - .NET 8 (LTS) - ASP.NET Core - EF Core with a **SQ...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ing: - .NET 8 (LTS) - ASP.NET Core - EF Core with a SQLite database for...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ith a SQLite database for simplicity - Docker Compose for basic containerizat...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...monstrating modern ASP.NET Core patterns - Complexity: Simple CRUD operations wit...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...operations with a single Player entity - Focus: Clean architecture, best practi...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...e, best practices, and maintainable code - Database: SQLite for development; Post...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...entions Follow standard C# conventions: - Use PascalCase for class names, method...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...nionated) ## 🏗️ Architectural Patterns This project follows a **layered archite...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ration of concerns: ### Layer Structure - Controllers (/Controllers) - Handle ...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...layerResponseModel - API response model - **Validators** (/Validators`) - FluentVal...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...database concerns ### Design Principles - Dependency Injection for all services ...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...tion** for all services and repositories - Repository Pattern for data access abs...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...ry Pattern** for data access abstraction - Service Layer for business logic encap...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...Layer** for business logic encapsulation - AutoMapper for clean object transforma...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...apper** for clean object transformations - FluentValidation for robust input vali...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...Validation** for robust input validation - Async/Await throughout the application...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...diomatic ASP.NET Core controller actions - Writing EF Core queries using LINQ - Fol...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...ons - Writing EF Core queries using LINQ - Following async programming practices - ...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ... - Following async programming practices - Producing unit tests using xUnit - S...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ... Suggesting dependency-injected services - Adhering to RESTful naming and HTTP stat...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ... to RESTful naming and HTTP status codes - Using ILogger<T> for logging - Working...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...s codes - Using ILogger<T> for logging - Working with Docker-friendly patterns - ...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ... - Working with Docker-friendly patterns - Implementing proper error handling and v...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...ing proper error handling and validation - Using appropriate HTTP status codes (200...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...us codes (200, 201, 400, 404, 409, etc.) - Following the existing caching patterns ...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...MemoryCache` ## 🚫 Copilot Should Avoid - Generating raw SQL unless explicitly req...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...ating raw SQL unless explicitly required - Using EF Core synchronous APIs (e.g., `F...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...stOrDefaultoverFirstOrDefaultAsync`) - Suggesting static service or repository ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...ing static service or repository classes - Including XML comments or doc stubs unle...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...L comments or doc stubs unless requested - Suggesting patterns that conflict with D...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...ice()instead of constructor injection) - UsingConfigureAwait(false)` in ASP.NET...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...reAwait(false)` in ASP.NET Core contexts - Implementing complex inheritance hierarc...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ... hierarchies when composition is simpler - Adding unnecessary middleware or filters...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ers without clear purpose ## 🧪 Testing - Use xUnit - Use Moq for mocking ...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...vior** - Place unit tests under test/ following structure already present (e.g., `Unit/...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...ng patterns ## 🔧 Tooling & Environment - Format code with CSharpier - SQLite ...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...files ## 🏷️ Technology Stack Deep Dive ### Entity Framework Core - DbContext: `...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...ack Deep Dive ### Entity Framework Core - DbContext: PlayerDbContext with SQLi...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...: PlayerDbContext with SQLite provider - Migrations: Used for schema management...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...d for schema management and data seeding - Pooling: AddDbContextPool for better...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...AddDbContextPoolfor better performance - **Query Optimization**:AsNoTracking()` f...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...AsNoTracking()` for read-only operations - Async Operations: All database calls u...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...se calls use async/await ### AutoMapper - Profile: PlayerMappingProfile handle...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...ed in DI container ### FluentValidation - Validators: `PlayerRequestModelValidat...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...uestModelValidator` for input validation - Integration: Automatic validation in c...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...idation in controllers before processing - Error Messages: Descriptive validation...
(QB_NEW_EN)
[grammar] ~124-~124: There might be a mistake here.
Context: ...alidation messages ### Caching Strategy - IMemoryCache: Service-level caching fo...
(QB_NEW_EN)
[grammar] ~125-~125: There might be a mistake here.
Context: ...ervice-level caching for read operations - Cache Keys: Consistent naming with `na...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...onsistent naming with nameof() pattern - Invalidation: Cache cleared on data mo...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...n**: Cache cleared on data modifications - TTL: Sliding expiration (10 min) + abs...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...ging**: Consistent log message templates - Log Levels: Appropriate use of Informa...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...riate use of Information, Warning, Error - Context: Include relevant data in log ...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...*: Include relevant data in log messages - Configuration: File and console sinks ...
(QB_NEW_EN)
[grammar] ~146-~146: There might be a mistake here.
Context: ...d helper logic ## 🧘 General Philosophy Keep things **simple, clear, and idiomat...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ... ## 📋 Common Patterns in This Codebase ### Repository Pattern ```csharp // Generic ...
(QB_NEW_EN)
[grammar] ~193-~193: There might be a mistake here.
Context: ...ed logging } ``` ## 🎯 Domain Knowledge ### Player Entity Context - *Squad Numbers...
(QB_NEW_EN)
[grammar] ~195-~195: There might be a mistake here.
Context: ...ain Knowledge ### Player Entity Context - Squad Numbers: Must be unique (1-99 ty...
(QB_NEW_EN)
[grammar] ~201-~201: There might be a mistake here.
Context: ...d league information ### Business Rules - Squad numbers cannot be duplicated - All...
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ...the stack ## 🚨 Error Handling Patterns csharp // Controller level - return appropriate HTTP status codes if (await playerService.RetrieveBySquadNumberAsync(squadNumber) != null) { return TypedResults.Conflict($"Squad number {squadNumber} already exists"); } // Service level - structured logging logger.LogWarning("Player with squad number {SquadNumber} not found", squadNumber); // Repository level - null handling var player = await _dbSet.FirstOrDefaultAsync(p => p.Id == id); return player; // Let caller handle null ## 🔄 Data Flow Patterns 1. **Request Flow...
(QB_NEW_EN)
[grammar] ~224-~224: There might be a mistake here.
Context: ...andle null ``` ## 🔄 Data Flow Patterns 1. Request Flow: Controller → Validation ...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...dation → Service → Repository → Database 2. Response Flow: Database → Repository →...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...rvice → AutoMapper → Controller → Client 3. Caching: Service layer implements `IMe...
(QB_NEW_EN)
[grammar] ~231-~231: There might be a mistake here.
Context: ... ## 🎯 When to Use Different Approaches ### Choose EF Core When: - Simple CRUD opera...
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...ent Approaches ### Choose EF Core When: - Simple CRUD operations (current use case...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ...imple CRUD operations (current use case) - Rapid development needed - Strong typing...
(QB_NEW_EN)
[grammar] ~235-~235: There might be a mistake here.
Context: ...ent use case) - Rapid development needed - Strong typing and compile-time checking ...
(QB_NEW_EN)
[grammar] ~236-~236: There might be a mistake here.
Context: ...ping and compile-time checking preferred - Schema migrations are important ### Con...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...rtant ### Consider Raw SQL/Dapper When: - Complex queries with performance require...
(QB_NEW_EN)
[grammar] ~240-~240: There might be a mistake here.
Context: ...ex queries with performance requirements - Need fine-grained control over SQL - Wor...
(QB_NEW_EN)
[grammar] ~241-~241: There might be a mistake here.
Context: ...nts - Need fine-grained control over SQL - Working with existing stored procedures ...
(QB_NEW_EN)
[grammar] ~242-~242: There might be a mistake here.
Context: ... Working with existing stored procedures - Micro-service with minimal ORM overhead ...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...RM overhead ## 🌐 API Design Guidelines - Use proper HTTP verbs (GET, POST, PUT, D...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...s ## 🚀 Future Evolution Considerations - Database Migration: SQLite → PostgreSQ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.github/copilot-instructions.md
233-233: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
239-239: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ 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). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (csharp)



Add comprehensive .github/copilot-instructions.md with detailed guidance for AI assistance including:
Closes #271
Summary by CodeRabbit