-
Notifications
You must be signed in to change notification settings - Fork 658
[CLI] Add --framework option to aspire new #9985
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
base: main
Are you sure you want to change the base?
[CLI] Add --framework option to aspire new #9985
Conversation
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 adds a --framework
option to the aspire new
command so that users can specify a target framework when scaffolding new projects.
- Introduces a nullable
--framework
CLI option and propagates it through parsing and into thedotnet new
invocation. - Updates the
IDotNetCliRunner
interface, its real implementation, and the test stub to include the optional framework parameter. - Adds resource entries and translations for the new argument, and a test verifying that the framework value is passed correctly.
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Aspire.Cli/Commands/NewCommand.cs | Added --framework CLI option with description |
src/Aspire.Cli/Templating/DotNetTemplateFactory.cs | Read framework from parse result and pass it to the runner |
src/Aspire.Cli/DotNetCliRunner.cs | Extended NewProjectAsync to include framework in CLI args |
tests/Aspire.Cli.Tests/TestServices/TestDotNetCliRunner.cs | Adjusted test stub signature to accept the framework parameter |
tests/Aspire.Cli.Tests/Commands/NewCommandTests.cs | Added test for framework propagation |
src/Aspire.Cli/Resources/NewCommandStrings.resx & .xlf | Added FrameworkArgumentDescription and translations |
Files not reviewed (1)
- src/Aspire.Cli/Resources/NewCommandStrings.Designer.cs: Language not supported
Comments suppressed due to low confidence (3)
tests/Aspire.Cli.Tests/TestServices/TestDotNetCliRunner.cs:18
- The
NewProjectAsyncCallback
delegate signature omits theextraArgs
parameter, so any additional CLI flags are ignored in tests; consider adding astring[] extraArgs
parameter afterframework
to mirror the production signature.
public Func<string, string, string, string?, DotNetCliRunnerInvocationOptions, CancellationToken, int>? NewProjectAsyncCallback { get; set; }
src/Aspire.Cli/Templating/DotNetTemplateFactory.cs:200
- [nitpick] Using a literal string to retrieve the option value can be fragile; consider storing the
Option<string?>
instance in a field and usingparseResult.GetValueForOption(frameworkOption)
to tie it directly to the declared option.
var framework = parseResult.GetValue<string?>("--framework");
tests/Aspire.Cli.Tests/Commands/NewCommandTests.cs:574
- You added a test for when a framework is specified; consider adding a complementary test to verify that, when no
--framework
flag is provided, the default (null or empty) framework value is passed through correctly.
[Fact]
I'm not sure we want this change. |
@davidfowl I was talking to @DamianEdwards and @maddymontaquila about this. I think we resolved that did want to ask what the target framework should be somewhere in the flow of questions. We would want the latest stable build to be the default, but allow us to choose 8.0 or 10.0 preview too. |
Did we decide that we are going to flow options wholestale to the template provider? |
Well, I think that there are two things here. I think we agreed that aspire new should really be primarily an interactive experience. So to that end adding the |
I'm ok with prompting for the framework. I was more worried that we didn't have a plan for flowing arguments cleanly. |
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #9931
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template