Repository navigation
Issue/736 add more warnings as errors - #1052
Open
MartinHock wants to merge 1 commit into
Open
MartinHock wants to merge 1 commit into
MartinHock wants to merge 1 commit into
Conversation
MartinHock
force-pushed
the
issue/736-add-more-warnings-as-errors
branch
2 times, most recently
from
July 20, 2026 15:29
97f2598 to
6651e94
Compare
Contributor
Author
|
The current cross-platform build succeeds on Ubuntu, Windows, and macOS. However, merging is still blocked because the repository requires a status check named I found that this required check appears to come from the former jobs: The current workflow reports the matrix jobs successfully, but no longer reports a check named exactly Could a maintainer please update the required status check in the branch protection rules, or add an appropriate aggregate |
MartinHock
force-pushed
the
issue/736-add-more-warnings-as-errors
branch
from
August 17, 2026 18:49
4e57f21 to
a4bf844
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the template to build cleanly with stricter analyzer settings (latest-all + warnings-as-errors), largely by addressing newly surfaced code analysis warnings across src, tests, and CI configuration.
Changes:
- Enable
AnalysisLevel=latest-Alland treat code analysis warnings as errors; align solution/projects with .NET 10 + updated test tooling. - Fix/avoid analyzer warnings via naming/constant updates, cancellation-token usage, invariant-culture formatting, logging template tweaks, and minor code cleanups.
- Update test/CI/dependency management (new test settings files, workflow adjustments, dependabot grouping, package additions/pins).
Reviewed changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Clean.Architecture.UnitTests/NoOpMediator.cs | Analyzer-driven tweaks (static suggestion + cancellation token use). |
| tests/Clean.Architecture.IntegrationTests/Data/EfRepositoryUpdate.cs | Uses fixture DbContext accessor to avoid field accessibility warnings. |
| tests/Clean.Architecture.IntegrationTests/Data/BaseEfRepoTestFixture.cs | Adds dispose patterns and encapsulates DbContext to satisfy analyzers. |
| tests/Clean.Architecture.IntegrationTests/Clean.Architecture.IntegrationTests.csproj | Adds package reference(s) to satisfy dependency/tooling needs. |
| tests/Clean.Architecture.FunctionalTests/CustomWebApplicationFactory.cs | Corrects dispose override and logging template property naming. |
| tests/Clean.Architecture.FunctionalTests/Clean.Architecture.FunctionalTests.csproj | Adds package reference(s) used by test infrastructure/container tooling. |
| tests/Clean.Architecture.FunctionalTests/ApiEndpoints/ContributorList.cs | Updates to renamed SeedData constant. |
| tests/Clean.Architecture.AspireTests/Clean.Architecture.AspireTests.csproj | Aligns target framework with solution-wide net10.0. |
| tests/.runsettings | Adds parallel test execution settings. |
| tests/.editorconfig | Adds test-scoped editorconfig to align formatting/analyzer settings. |
| src/Clean.Architecture.Web/Contributors/Update.UpdateContributorValidator.cs | Removes unused using + updates to renamed schema constant. |
| src/Clean.Architecture.Web/Contributors/Update.UpdateContributorRequest.cs | Uses invariant culture formatting + explicit string comparison for route building. |
| src/Clean.Architecture.Web/Contributors/Update.cs | Removes unused using. |
| src/Clean.Architecture.Web/Contributors/List.cs | Renames constants and threads cancellation token into mediator/send calls. |
| src/Clean.Architecture.Web/Contributors/GetById.GetContributorByIdRequest.cs | Uses invariant culture formatting + explicit string comparison for route building. |
| src/Clean.Architecture.Web/Contributors/GetById.cs | Updates use-case namespace import. |
| src/Clean.Architecture.Web/Contributors/Delete.DeleteContributorRequest.cs | Uses invariant culture formatting + explicit string comparison for route building. |
| src/Clean.Architecture.Web/Contributors/Create.cs | Threads cancellation token into mediator call; minor DTO cleanup. |
| src/Clean.Architecture.Web/Configurations/MiddlewareConfig.cs | Formatting + logging template property naming adjustments. |
| src/Clean.Architecture.Web/Configurations/LoggerConfigs.cs | Uses invariant culture format provider for console sink. |
| src/Clean.Architecture.UseCases/Contributors/Update/UpdateContributorHandler.cs | Standardizes cancellation token naming/usage. |
| src/Clean.Architecture.UseCases/Contributors/List/ListContributorsQuery.cs | Updates default paging constant name. |
| src/Clean.Architecture.UseCases/Contributors/List/ListContributorsHandler.cs | Updates default paging constant name. |
| src/Clean.Architecture.UseCases/Contributors/GetContributor/GetContributorQuery.cs | Moves query into GetContributor namespace. |
| src/Clean.Architecture.UseCases/Contributors/Get/GetContributorHandler.cs | Adapts handler to new query namespace + primary constructor usage. |
| src/Clean.Architecture.UseCases/Constants.cs | Makes constants container static and renames constants to PascalCase. |
| src/Clean.Architecture.ServiceDefaults/Extensions.cs | Adds pragma suppression for intentional commented sample code + string comparison options. |
| src/Clean.Architecture.Infrastructure/Email/MimeKitEmailSender.cs | Renames parameters and adjusts logging; analyzer-driven changes. |
| src/Clean.Architecture.Infrastructure/Email/FakeEmailSender.cs | Renames parameters and adjusts logging. |
| src/Clean.Architecture.Infrastructure/Data/SeedData.cs | Renames constant to PascalCase and updates loop usage. |
| src/Clean.Architecture.Infrastructure/Data/EventDispatcherInterceptor.cs | Minor tweak to avoid analyzer warning around Any() usage. |
| src/Clean.Architecture.Infrastructure/Data/Config/VogenEfCoreConverters.cs | Seals converter type per analyzer guidance. |
| src/Clean.Architecture.Infrastructure/Data/Config/DataSchemaConstants.cs | Renames constant to PascalCase. |
| src/Clean.Architecture.Infrastructure/Clean.Architecture.Infrastructure.csproj | Adds SQLite bundle dependency to support runtime/native requirements. |
| src/Clean.Architecture.Core/Services/DeleteContributorService.cs | Uses precompiled logging delegate and improves XML docs to satisfy analyzers. |
| src/Clean.Architecture.Core/Interfaces/IEmailSender.cs | Renames parameters for clarity/consistency. |
| src/Clean.Architecture.Core/ContributorAggregate/Handlers/ContributorNameUpdatedEmailNotificationHandler.cs | Logging template property naming adjustment. |
| src/Clean.Architecture.Core/ContributorAggregate/Handlers/ContributorDeletedHandler.cs | Logging template property naming adjustment. |
| sample/tests/NimblePros.SampleToDo.FunctionalTests/NimblePros.SampleToDo.FunctionalTests.csproj | Adds package reference(s) aligned with updated test tooling. |
| sample/src/NimblePros.SampleToDo.Infrastructure/NimblePros.SampleToDo.Infrastructure.csproj | Adds SQLite native lib dependency. |
| sample/src/NimblePros.SampleToDo.AspireHost/NimblePros.SampleToDo.AspireHost.csproj | Adds MessagePack and minor XML formatting cleanup. |
| sample/Directory.Packages.props | Adds/pins new package versions (including SSH.NET/MessagePack) and updates Aspire package version. |
| MinimalClean/src/MinimalClean.Architecture.AspireHost/MinimalClean.Architecture.AspireHost.csproj | Adds MessagePack reference. |
| MinimalClean/Directory.Packages.props | Expands/pins package versions for MinimalClean template. |
| global.json | Configures test runner to Microsoft.Testing.Platform. |
| Directory.Packages.props | Updates and adds centrally-managed package versions (incl. xUnit v4 + docker/testcontainers pins). |
| Directory.Build.props | Enables latest-all analysis and treats code-analysis warnings as errors. |
| Clean.Architecture.slnx | Adds extra solution items (editorconfig/gitignore/tests editorconfig). |
| .gitignore | Adds ignores for SonarQube working dir and local state folders. |
| .github/workflows/cross-platform-build-test.yml | Updates test invocation/reporting and artifact paths. |
| .github/dependabot.yml | Adds structured dependabot groups/schedule/labels/commit message formatting. |
| .editorconfig | Adds analyzer suppressions and related comments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
26
to
27
| await client.DisconnectAsync(true, | ||
| new CancellationToken(canceled: true)); |
| public async ValueTask Handle(ContributorDeletedEvent domainEvent, CancellationToken cancellationToken) | ||
| { | ||
| logger.LogInformation("Handling Contributed Deleted event for {contributorId}", domainEvent.ContributorId); | ||
| logger.LogInformation("Handling Contributed Deleted event for {ContributorId}", domainEvent.ContributorId); |
Comment on lines
+150
to
+152
| // CA1848: Use 'LoggerMessage.Define' instead of 'LoggerMessage.Define<T>' | ||
| // DO not force Logger Massages Delegates all everywhere | ||
| dotnet_diagnostic.CA1848.severity= none |
| { | ||
| public const string Route = "/Contributors/{ContributorId:int}"; | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString()); | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString(CultureInfo.InvariantCulture), StringComparison.InvariantCulture); |
| { | ||
| public const string Route = "/Contributors/{ContributorId:int}"; | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString()); | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString(CultureInfo.InvariantCulture), StringComparison.InvariantCulture); |
| { | ||
| public const string Route = "/Contributors/{ContributorId:int}"; | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString()); | ||
| public static string BuildRoute(int contributorId) => Route.Replace("{ContributorId:int}", contributorId.ToString(CultureInfo.InvariantCulture), StringComparison.InvariantCulture); |
MartinHock
force-pushed
the
issue/736-add-more-warnings-as-errors
branch
from
August 17, 2026 18:55
a5460c2 to
fda32e1
Compare
MartinHock
force-pushed
the
issue/736-add-more-warnings-as-errors
branch
from
August 23, 2026 16:43
bfac1b7 to
003d186
Compare
This branch has not been deployed
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
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.
removed the warnings that were appearing after latest-all was added to Directoy.build.props
and analysis was switched on