-
Notifications
You must be signed in to change notification settings - Fork 703
[release/9.3] Respond to API Review feedback #9285
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
…ainerRegistry is experimental.
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.
Pull Request Overview
This PR addresses API review feedback by refining the public API surface and consolidating namespaces while removing unused or redundant references.
- Removed obsolete using directives in test files.
- Added or updated Experimental attributes and suppression pragmas in production code.
- Updated access modifiers and namespaces to streamline the API design.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/Aspire.Hosting.Azure.Tests/AzureContainerRegistryTests.cs | Removed unused using directive. |
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs | Removed unused using directive. |
tests/Aspire.Hosting.Azure.Tests/AzureAppServiceTests.cs | Removed unused using directive. |
src/Aspire.Hosting/ResourceBuilderExtensions.cs | Added Experimental attribute for publishing callback. |
src/Aspire.Hosting/Publishing/PublishingContext.cs | Added using statement and Experimental attribute. |
src/Aspire.Hosting/Publishing/Publisher.cs | Added pragma suppression for Experimental API warning. |
src/Aspire.Hosting/PublisherDistributedApplicationBuilderExtensions.cs | Changed API access from public to internal and removed the Experimental attribute. |
src/Aspire.Hosting/CompatibilitySuppressions.xml | Updated suppressions for removed experimental publishers. |
src/Aspire.Hosting/ApplicationModel/PublishingCallbackAnnotation.cs | Added using statement and Experimental attribute. |
src/Aspire.Hosting/ApplicationModel/ContainerRegistryReferenceAnnotation.cs | Added using statement and Experimental attribute. |
src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs | Added pragma suppression for Experimental API warning. |
src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs | Added pragma suppression for Experimental API warning within publish method. |
src/Aspire.Hosting.Azure/AzurePublishingContext.cs | Converted a public field to an immutable property. |
src/Aspire.Hosting.Azure/AzureEnvironmentResourceExtensions.cs | Updated using directives and namespace declaration. |
src/Aspire.Hosting.Azure.Storage/AzureBlobStorageContainerResource.cs | Updated namespace from Aspire.Hosting to Aspire.Hosting.Azure. |
src/Aspire.Hosting.Azure.ContainerRegistry/AzureContainerRegistryResource.cs | Normalized pragma format and updated namespace. |
src/Aspire.Hosting.Azure.ContainerRegistry/AzureContainerRegistryExtensions.cs | Removed unused using directive. |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs | Updated namespace from Azure.AppService to Azure. |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentExtensions.cs | Introduced pragma suppression around annotation use. |
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs | Introduced pragma suppression around annotation use. |
Can you also make the publishing apis experimental (annotation and WithPublishingCallback?) |
I am making them experimental: aspire/src/Aspire.Hosting/Publishing/PublishingContext.cs Lines 18 to 19 in f092e67
aspire/src/Aspire.Hosting/ResourceBuilderExtensions.cs Lines 294 to 295 in f092e67
|
Customer Impact
Adjusting our public APIs before release, so we don't have to make breaking changes, or live with unwanted API shape.
See API review comments on #8736.
Testing
Unit tests.
Risk
Low since these APIs are either experiemental or haven't shipped yet.
Regression?
No