Skip to content

Conversation

joperezr
Copy link
Member

@joperezr joperezr commented Aug 28, 2024

Description

When making the workload changes, I ran the xml formatter on the SDK.targets file to format tabs/spaces and indentation correctly, but it looks like that made some formatting changes that introduced bugs like adding newlines to property values, which would then break conditions when doing string comparisons. Thanks a lot @eerhardt for debugging and finding this out. The changes in this PR revert all the formatting changes made in that PR, which means the net difference since 8.1 will be the addition of the new target.

Fixes #5436

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
    • 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
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@joperezr joperezr requested a review from eerhardt August 28, 2024 16:31
@ghost ghost added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Aug 28, 2024
@joperezr
Copy link
Member Author

After applying the changes in this PR locally, I can see that the issue is resolved:

image

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this!

@joperezr
Copy link
Member Author

/backport to release/8.2

Copy link
Contributor

Started backporting to release/8.2: https://github.com/dotnet/aspire/actions/runs/10601707713

@v-elenafeng
Copy link

@joperezr Is this ready to be merged? We're still experiencing the issue with 9.0 P4.

@eerhardt eerhardt merged commit ab0a8d5 into dotnet:main Sep 4, 2024
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2024
@joperezr joperezr deleted the FixVSWarnings branch April 1, 2025 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

[WebToolsE2E][Aspire] After creating an Aspire project, some warning icons are displayed in Dependencies | Projects of the AppHost project.
3 participants