-
Notifications
You must be signed in to change notification settings - Fork 693
Refactor templates into single template groups that allow you select TFM and aspire version #6365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
70461d1
to
47621fb
Compare
<!-- for templates --> | ||
<MicrosoftExtensionsHttpResiliencePackageVersionForNet8>$(MicrosoftExtensionsHttpResiliencePackageVersion)</MicrosoftExtensionsHttpResiliencePackageVersionForNet8> | ||
<MicrosoftExtensionsHttpResiliencePackageVersionForNet9>$(MicrosoftExtensionsHttpResiliencePackageVersion)</MicrosoftExtensionsHttpResiliencePackageVersionForNet9> | ||
<MicrosoftExtensionsHttpResiliencePackageVersionForNet9>9.0.0-preview.9.24507.7</MicrosoftExtensionsHttpResiliencePackageVersionForNet9> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/Aspire.ProjectTemplates/templates/aspire-apphost/.template.config/template.json
Outdated
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-apphost/.template.config/template.json
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-empty/.template.config/template.json
Outdated
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-mstest/.template.config/template.json
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-nunit/.template.config/template.json
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-servicedefaults/.template.config/template.json
Outdated
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-xunit/.template.config/template.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this 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
src/Aspire.ProjectTemplates/templates/aspire-apphost/.template.config/template.json
Outdated
Show resolved
Hide resolved
src/Aspire.ProjectTemplates/templates/aspire-empty/.template.config/ide.host.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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:
Checklist
<remarks />
and<code />
elements on your triple slash comments?Microsoft Reviewers: Open in CodeFlow