Skip to content

Improve XML comments for CustomResourceSnapshot.HealthReports property - #19930

Open
Pasi Savolainen (pasisavolainen) wants to merge 1 commit into
microsoft:mainfrom
pasisavolainen:patch-1
Open

Improve XML comments for CustomResourceSnapshot.HealthReports property#19930
Pasi Savolainen (pasisavolainen) wants to merge 1 commit into
microsoft:mainfrom
pasisavolainen:patch-1

Conversation

@pasisavolainen

Copy link
Copy Markdown

Hint for CustomResourceSnapshot.HealthReports property updating mechanism.

Description

HealthReports appears to be internal init without clear mechanism on how to update, but there is this extension that is only discoverable by browsing sources

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • [n] No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • [n] No

Hint for HealthReports property updating mechanism.
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
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/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19930

Or

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

@github-actions github-actions Bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Sep 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Only a non-blocking documentation wording nit remains.

Pull request overview

Improves discoverability of the supported health-report update mechanism.

Changes:

  • Links HealthReports documentation to WithHealthReports.
File summaries
File Review
src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs Adds update guidance; minor wording nit: clarify that WithHealthReports returns a new snapshot rather than modifying the original.
Review details

Suppressed comments (1)

src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs:91

  • This wording implies that WithHealthReports modifies the existing snapshot, but the method returns a new record copy and leaves the original unchanged. Describe the copy semantics so callers do not discard the returned value.
    /// Use <see cref="CustomResourceSnapshotExtensions.WithHealthReports"/> to modify.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

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

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants