Mark IDeveloperCertificateService experimental #12648
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12648Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12648" |
There was a problem hiding this comment.
Pull Request Overview
This PR marks the IDeveloperCertificateService interface as experimental by adding the [Experimental] attribute with diagnostic ID ASPIRECERTIFICATES001. To address resulting compiler warnings, #pragma warning disable/restore directives are added throughout the codebase wherever this interface is referenced or implemented.
Key Changes:
- Added
[Experimental]attribute toIDeveloperCertificateServiceinterface - Suppressed ASPIRECERTIFICATES001 warnings in all files that reference or implement this interface
- Added
using System.Diagnostics.CodeAnalysis;to support the Experimental attribute
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/IDeveloperCertificateService.cs | Added Experimental attribute and CodeAnalysis using directive |
| src/Aspire.Hosting/DeveloperCertificateService.cs | Suppressed warning on class implementing the experimental interface |
| src/Aspire.Hosting/DistributedApplicationBuilder.cs | Suppressed warning where service is registered in DI container |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Suppressed warnings on field declaration and constructor parameter |
| src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs | Suppressed warning where service is retrieved from DI |
| src/Aspire.Hosting.Yarp/YarpResourceExtensions.cs | Suppressed warning where service is retrieved from DI |
| tests/Aspire.Hosting.Tests/Utils/TestDeveloperCertificateService.cs | Suppressed warning on test implementation class |
| tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs | Suppressed warning where service is retrieved in test |
| tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs | Suppressed warning around DcpExecutor constructor call in test |
| tests/Aspire.Hosting.Yarp.Tests/AddYarpTests.cs | Suppressed warnings in multiple test methods that create builders using the experimental service |
| [InlineData(false)] | ||
| public async Task VerifyRunEnvVariablesAreSet(bool containerCertificateSupport) | ||
| { | ||
| #pragma warning disable ASPIRECERTIFICATES001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. |
There was a problem hiding this comment.
The pragma warning disable could be placed at the file scope (after the using statements) rather than within individual test methods. This would reduce repetition across 5 test methods and make the code more maintainable. The experimental nature of IDeveloperCertificateService applies to the entire test file.
| #pragma warning disable ASPIRECERTIFICATES001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. | ||
| private readonly IDeveloperCertificateService _developerCertificateService; | ||
| #pragma warning restore ASPIRECERTIFICATES001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. |
There was a problem hiding this comment.
The field declaration and constructor parameter both suppress the same warning. Consider placing the pragma at file scope to avoid duplicating the suppression for both the field (lines 73-75) and constructor parameter (lines 110-112).
|
/backport to release/13.0 |
|
Started backporting to release/13.0: https://github.com/dotnet/aspire/actions/runs/19056239122 |
|
@danegsta - can you open a new docs issue for this new diagnostic - New |
Done: dotnet/docs-aspire#5449 |
3 participants
Description
Marks the new IDeveloperCertificateService API as experimental in case we need to make some tweaks to it as we update our developer certificate support.