Rename WaitBehavior enumeration values for clarity#7703
Rename WaitBehavior enumeration values for clarity#7703mitchdenny wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/Aspire.Hosting/ApplicationModel/WaitAnnotation.cs:68
- The comment should be updated to reflect the new behavior accurately. Suggestion: 'Throw an exception if the resource enters a terminal state.'
/// Throw if the resource enters a terminal state.
src/Aspire.Hosting/ApplicationModel/ResourceNotificationService.cs:15
- The error message could be more descriptive. Consider updating it to: "The operation was cancelled before the resource '{resourceName}' met the predicate condition."
throw new OperationCanceledException("The operation was cancelled before the resource met the predicate condition.");
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
eerhardt
left a comment
There was a problem hiding this comment.
I know I commented #7374 (comment) that "WaitBehavior.Throw" is a little oxymoronic, but this is used in WaitFor and WaitAnnotation, so no other name really makes sense for the enum.
This change is better than what we have. So LGTM.
|
/backport to release/9.1 |
|
Started backporting to release/9.1: https://github.com/dotnet/aspire/actions/runs/13449758642 |
|
@mitchdenny Is this PR still happening? What was the final outcome of bike shedding? |
|
No just closed it and the backport. But this one is going in: #7728 |
|
Once it merges to main I backport I had a build failure and Dan make a suggestion so just doing a build loop again. |
Update enumeration values to improve understanding of their intent, changing
WaitOnResourceUnavailabletoWaitandStopOnResourceUnavailabletoThrow. Adjust related code to reflect these changes.