Skip to content

Conversation

@jfversluis
Copy link
Member

Description

The KnownResourceStateStyle has a wrong value of warn, where the dashboard is expecting warning. I guess we might want to change KnownResourceStateStyle.Warn then also to Warning? Or the other way around and make the dashboard look for "warn" instead of "warning" let me know if you want to change it the other way around.

https://github.com/dotnet/aspire/blob/main/src/Aspire.Dashboard/Model/ResourceStateViewModel.cs#L77

@Copilot Copilot AI review requested due to automatic review settings October 22, 2025 14:00
@github-actions
Copy link
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12278

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12278"

Copy link
Contributor

Copilot AI left a 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 fixes a mismatch between the resource state string used in Aspire.Hosting and what the Dashboard expects. The KnownResourceStateStyle.Warn constant was incorrectly set to "warn" but the Dashboard looks for "warning".

Key Changes:

  • Updated the Warn constant value from "warn" to "warning" to align with Dashboard expectations

public static readonly string Info = "info";

/// <summary>
/// The warn state. Useful for showing warnings.
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML documentation comment says 'The warn state' but the value is now 'warning'. Consider updating the comment to match: 'The warning state. Useful for showing warnings.' to maintain consistency between the member name, its value, and the documentation.

Suggested change
/// The warn state. Useful for showing warnings.
/// The warning state. Useful for showing warnings.

Copilot uses AI. Check for mistakes.
@afscrome
Copy link
Contributor

I did try to fix this in #11994 by changing the state int he dashboard, but it caused some test failures so I think changing KnownResourceStateStyle is safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants