Blueprint Code Review
🔴 Block merge
- Nullable suppressed:
!operator used to silence a nullable warning — fix the root cause - Logic in controller: business logic placed directly in a controller instead of an
IRequestHandler<T>— move it to the handler - API contract changed without regenerating the NSwag TypeScript client (
npm run nswag) - Secret / credential committed to code or config
- Build warning introduced —
TreatWarningsAsErrorsis on; CI will fail - Generated Angular file edited manually — files under
src/app/features/produced bynpm run codegen:runmust never be edited by hand; update theCodeGeneration.Setupconfiguration instead - NuGet version added to a
.csproj— all versions must be declared inDirectory.Packages.propsonly
🟡 Requires discussion
- No tests for a new handler, validator, or domain rule
- Service registered via
IServiceCollectioninstead of an Autofac module inInfrastructure/Autofac/Modules/ .subscribe()used in Angular wherefirstValueFrom()would suffice (one-shot observable)- FluentValidation validator exists but not wired to DI, or missing entirely for a command/query that has user inputs
- Static
Log.*used in business code instead of injectedILogger<T> - Domain entity exposes public setters instead of using the
Create/Updatefactory methods - Non-standalone Angular component created without justification
🟢 Suggestions
- Separate
[Test]methods that differ only in literal values → collapse with[TestCase]/[TestCaseSource] - Separate
it()blocks that differ only in data → collapse withit.each nameof()not used where a string references a member name- Comment explains what the code does rather than why — remove or rewrite
- Explicit default argument passed to a helper that already defaults to that value
- Missing
[PublicAPI]on a DTO / request / response type, or missing[UsedImplicitly]on a handler or validator
Checklist
.NET
- All business logic in
IRequestHandler<T>— not in controllers or plain services - FluentValidation validator present for every command/query that has user inputs
- No
!null-forgiving operators; no new nullable warnings - New services registered in an Autofac module (not
IServiceCollection) - Serilog message templates used — no string interpolation in log calls
- Tests cover happy path + key validation/error paths
-
[TestCase]/[TestCaseSource]used wherever multiple tests share the same structure - NuGet
Version=not added to any.csproj— useDirectory.Packages.props -
[PublicAPI]on DTOs / request / response types;[UsedImplicitly]on handlers and validators - Domain entities: private setters,
Create(Command)factory,Update(Command)method
Angular / TypeScript
-
firstValueFrom()for single-value observables;.subscribe()only for true streams -
readonlyarrow-function properties for all class methods except Angular lifecycle hooks -
ngOnInit,ngOnDestroy, etc. declared as plain methods (not arrow properties) -
it.eachused for parameterised Jest tests instead of repeatedit()blocks - Generated files in
src/app/features/NOT edited manually - NSwag client regenerated if API contracts changed (
npm run nswag) - New i18n keys reuse
shared.*for generic labels; no duplicate keys