Skip to content

Conversation

DamianEdwards
Copy link
Member

@DamianEdwards DamianEdwards commented Oct 18, 2024

Description

This merges each template group into a single template that lets you select TFM and Aspire version.

Also addresses some outstanding content and dependency version issues.

Contributes to #6076

Some screenshots of this working in VS locally when I install the new template:

image

image

image

image

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?
Microsoft Reviewers: Open in CodeFlow

@DamianEdwards DamianEdwards force-pushed the damianedwards/single-templates branch from 70461d1 to 47621fb Compare October 18, 2024 00:28
<!-- for templates -->
<MicrosoftExtensionsHttpResiliencePackageVersionForNet8>$(MicrosoftExtensionsHttpResiliencePackageVersion)</MicrosoftExtensionsHttpResiliencePackageVersionForNet8>
<MicrosoftExtensionsHttpResiliencePackageVersionForNet9>$(MicrosoftExtensionsHttpResiliencePackageVersion)</MicrosoftExtensionsHttpResiliencePackageVersionForNet9>
<MicrosoftExtensionsHttpResiliencePackageVersionForNet9>9.0.0-preview.9.24507.7</MicrosoftExtensionsHttpResiliencePackageVersionForNet9>
Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that Microsoft.Extensions.Http.Resilience is on a "tip support" policy. Meaning that only the latest version is supported. (I think there maybe a nuance here where the 8.0 version is LTS, but I can't seem to find the public policy.)

Given the above, I think we should always be referencing the current/latest version of Microsoft.Extensions.Http.Resilience from our templates regardless of TFM chosen. Even if you are targeting net8.0, we would bring in the 9.0 version of Microsoft.Extensions.Http.Resilience.

See also: Use 8.0 era dependencies for non net9.0 TFMs (dotnet/extensions#5470)

cc @joperezr

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't our repo be using the 9.x version then too? Currently it's using the 8.x version.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. That's part of this as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I hold off on a change here until we get consensus?

Copy link
Member

Choose a reason for hiding this comment

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

I think there is an agreement here. As @eerhardt points out, we want to use the tip of this package both in net8 and net9. There is still the PR that he pointed out above which hasn't been merged but will be merged before 9.0 ships.

@dotnet-policy-service dotnet-policy-service bot added the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 18, 2024
Copy link
Member

@phenning phenning left a comment

Choose a reason for hiding this comment

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

SO yea, ignore my comments about missing icons and ide.host.json files - you just need to add AspireVersion to the ide.host.jsons

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 18, 2024
@DamianEdwards
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@phenning phenning left a comment

Choose a reason for hiding this comment

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

one typo, one suggestion

Copy link
Member

@phenning phenning left a comment

Choose a reason for hiding this comment

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

Good from an authoring metadata standpoint

@DamianEdwards
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DamianEdwards DamianEdwards enabled auto-merge (squash) October 18, 2024 23:39
@DamianEdwards DamianEdwards merged commit 703dc10 into main Oct 19, 2024
5 of 8 checks passed
@DamianEdwards DamianEdwards deleted the damianedwards/single-templates branch October 19, 2024 00:32
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants