-
Notifications
You must be signed in to change notification settings - Fork 648
Add extension methods to PublishingStep & PublishingTask for direct Complete/Update operations #9995
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
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.
Good start! Remove the StatusText
property on PublishingStep
and any APIs related to it.
Done! Removed the |
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.
Add a new API on PublishingStep
that will let me create a Task parented to it.
var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Initial status", CancellationToken.None)
... Added the var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Initial status", CancellationToken.None); The method includes proper error handling and comprehensive tests. (70d9c39) |
@copilot Can you update this branch with the latest changes from main and update the |
Updated branch with latest changes from main and updated the |
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.
Undo the changes in Aspire.Hosting.Azure and Aspire.Hosting.Docker.
Changes in Aspire.Hosting.Azure and Aspire.Hosting.Docker have been reverted to their original state. The new extension methods remain available but these components now use the original publishing APIs instead. (c4ce5b1) |
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
…kerComposePublishingContext to use new PublishingExtensions APIs Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
d5984ee
to
cbbbfc3
Compare
@davidfowl @mitchdenny This is rebased and ready for review. |
/// <summary> | ||
/// The progress reporter that created this step. | ||
/// </summary> | ||
internal IPublishingActivityProgressReporter? Reporter { get; set; } |
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.
The ctor is public for this class, but this property is internal...?
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.
Fixed the accessibility inconsistency by making the Reporter
properties public to match the public constructors. This ensures consistent API design where users can create instances directly and all public members are accessible. (173b7cb)
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
@@ -72,7 +72,7 @@ public sealed class PublishingStep(string id, string title) | |||
/// <summary> | |||
/// The progress reporter that created this step. | |||
/// </summary> | |||
internal IPublishingActivityProgressReporter? Reporter { get; set; } | |||
public IPublishingActivityProgressReporter? Reporter { get; set; } |
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.
Sorry, I meant to make the constructor internal, not make these public.
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.
Made the constructors internal and restored the Reporter properties to internal access. This ensures proper encapsulation where only the progress reporter implementations can create instances, while the extension methods can still access the Reporter property within the same assembly. (8db3dc5)
…ore internal Reporter properties Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
This PR adds extension methods to
PublishingStep
andPublishingTask
that allow users to directly perform completion and update operations without needing to manually interact with theIPublishingActivityProgressReporter
.Changes Made
New Extension Methods
Added
PublishingExtensions
class with the following methods:For PublishingStep:
UpdateStatusAsync(string statusText)
- Updates the step's status textSucceedAsync(string? message)
- Completes the step successfullyWarnAsync(string? message)
- Completes the step with a warningFailAsync(string? errorMessage)
- Completes the step with an errorFor PublishingTask:
UpdateStatusAsync(string statusText)
- Updates the task's status textSucceedAsync(string? message)
- Completes the task successfullyWarnAsync(string? message)
- Completes the task with a warningFailAsync(string? errorMessage)
- Completes the task with an errorInfrastructure Changes
Reporter
property to bothPublishingStep
andPublishingTask
to store reference to the creating progress reporterStatusText
property toPublishingStep
to support status updatesIPublishingActivityProgressReporter
withUpdateStepAsync
methodPublishingActivityProgressReporter
andNullPublishingActivityProgressReporter
to set reporter references and implement the new interface methodUsage Example
All existing tests continue to pass, and comprehensive tests have been added for the new extension methods.
Fixes #9994.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.