- Notifications
You must be signed in to change notification settings - Fork845
Add project name normalization to match aspire's code generator logic#6818
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…-aspire projects. Add execution tests for the fix.
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.
Pull Request Overview
This PR addresses build errors that occur when using the--aspire flag with project names containing dots or other non-identifier characters. The fix applies Aspire's project name normalization logic to templates to ensure generated code references match the normalized class names.
- Adds project name normalization transforms to template configuration
- Introduces a new test case to verify the fix works with project names containing dots
- Ensures consistency between template-generated code and Aspire's AppHost target behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| AIChatWebExecutionTests.cs | Adds test case for project names with dots to verify normalization fix |
| template.json | Implements project name normalization transforms using regex replacement |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
… flag. More test cases coverage.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
…ests. Make the Aspire project name theory conditional.
Add an [EnvironmentVariableSkipCondition] attribute for conditional tests. Make the Aspire project name theory conditional. Keep a single [Fact] that gives comprehensive coverage of the pattern for a single provider.
MackinnonBuck left a comment
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.
Just one piece of feedback but looks great!
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...ojectTemplates/Microsoft.Extensions.AI.Templates.IntegrationTests/AIChatWebExecutionTests.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
jeffhandley left a comment
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 idea removing_Web from the replacement token so that it doesn't have to be re-appended.
Since this is now a single-step regex replacement, I had look back at thehttps://github.com/dotnet/templating/wiki/Reference-for-template.json#derived-symbol docs to understand why this is aderived replacer vs. the samegenerator approach as is used in the Aspire template. It makes sense though--we need the project name to get plugged in and do post-processing over that derived symbol.
Looks good!
7200baa intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#6811
Apply project name transformation in the templates' generated code using the same normalization logic as in
Aspire.Hosting.AppHost.targets. This ensures that when the--aspireflag is used, the generatedAppHost/Program.csreferences normalized class names (e.g.,some_nameinstead ofsome.name), resolving build errors caused by mismatched type or namespace references.Add execution tests to verify the fix for templates with dots or other non-identifier characters in their names.
Microsoft Reviewers:Open in CodeFlow