Skip to content

Conversation

@am11
Copy link
Member

@am11 am11 commented Oct 7, 2021

Upgraded console template to pass args tot he runner, as it gets a bit confusing when the passed args have no effect on the generated project.

@AndreyAkinshin
Copy link
Member

I think we should wait with such a change until .NET 6 is officially released.

@am11
Copy link
Member Author

am11 commented Oct 8, 2021

Yup, actually the second part of this change (the pain point about args propagation) was more imminent. :)

Regarding version, I think ideally the template should pick up the latest SDK which dotnet in dotnet new benchmark --console-app command supports and set <TargetFramework> in the generated csproj accordingly, so we don't have to manually update the config each time a new .NET version is released.

I can repurpose the PR and drop version revving.

@AndreyAkinshin
Copy link
Member

Regarding version, I think ideally the template should pick up the latest SDK

Do you have any idea how to do this using the existing templating engine?

I can repurpose the PR and drop version revving.

Sounds good.

@am11 am11 force-pushed the feature/templates branch from 9f2e016 to ba39a6a Compare October 8, 2021 18:35
@am11 am11 changed the title Upgrade console template to net6.0 Pass args to runner in console template Oct 8, 2021
@am11
Copy link
Member Author

am11 commented Oct 8, 2021

I have reverted the net6.0 change, we can reintroduce it after 6.0 is shipped as you suggested.

Regarding the framework inference, I just realized that it is already supported 🙂, i.e.

# the default
$ dotnet new benchmark --console-app -n foo
$ cat foo/foo.csproj | grep TargetFramework
    <TargetFramework>net5.0</TargetFramework>

vs.

# with --framework or -f option
$ dotnet new benchmark --console-app -f net6.0 -n foo
$ cat foo/foo.csproj | grep TargetFramework
    <TargetFramework>net6.0</TargetFramework>

I think to make it more maintainable for default case (so we don't feel tempted to update it after each .NET release), one option would be to make it explicit for console-app and remove the default case:

          {
-            "condition": "(framework == '' && consoleApp == true)",
+            "condition": "(consoleApp == true)",
-            "value": "net5.0"
+            "value": ""
          },
          {
            "condition": "(framework == '' && consoleApp == false)",
            "value": "netstandard2.0"
          },
          {
            "condition": "(framework != '')",
            "value": ""
          }
        ]

the side-effect is that user experience is not too great as passing --console-app is already not so great (as non-runnable lib flavor should not be the default), so marking --framework mandatory on top would be distasteful. 😁

@YegorStepanov YegorStepanov mentioned this pull request Jun 21, 2023
@AndreyAkinshin
Copy link
Member

Closed in favor of #2338

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants