Update implicit versioning logic for patches #346
Conversation
|
The plan is to have the CLI embed the default version in the BundleVersions.props. This is a little different from the dotnet SDK where the default versions are carried in the SDK instead of the CLI but I think keeping both default versions (i.e. lowest versions used in portable apps) and latest versions (i.e. highest versions used in standalone deployments) in the same place in the CLI seems better. |
| <PropertyGroup> | ||
| <!-- These properties will allow the latest patch versions to be set in the CLI (via BundledVersions.props) or overridden by tests --> | ||
| <DefaultPatchVersionForAspNetCoreAll2_1 Condition="'$(DefaultPatchVersionForAspNetCoreAll2_1)' == ''">2.1.1</DefaultPatchVersionForAspNetCoreAll2_1> | ||
| <DefaultPatchVersionForAspNetCoreApp2_1 Condition="'$(DefaultPatchVersionForAspNetCoreApp2_1)' == ''">2.1.1</DefaultPatchVersionForAspNetCoreApp2_1> |
There was a problem hiding this comment.
Shouldn't this be 2.1.0? The "latest" version should come from the bundled versions and be 2.1.1 once we patch, but the default version would still be 2.1.0.
There was a problem hiding this comment.
Assuming this meets the bug bar, we intentionally want to raise the default baseline to 2.1.1 due to aspnet/Universe#1180
…efault implicit version
|
@natemcmaster so with the logic you added, we wouldn't need to set the version again in the CLI anymore? As in we wouldn't need https://github.com/dotnet/cli/pull/9374/files? |
|
Sorry, I didn't see that you had opened another PR. I read dotnet/cli#9374 and I think you're right. I don't think we need to make changes to the CLI. |
|
I'm okay with splitting the version numbers (latest patch in cli and default patch in sdk) since this is what the dotnet sdk does. However, I did recall us having the discussion and decided that the version numbers (including setting the default versions to 2.1.1) should all reside in cli? Did that change? |
|
I guess I didn't think about it much after our last conversation. I just updated this PR to match your updates to aspnet/templating. We wanted to get these merged since we're planning to lock down an escrow build tomorrow. Yes, this is different than what I said and IMO still less than ideal, but I don't think it's a big deal either way. What do you think we should do? |
|
This is fine, I was just curious why we decided to do the opposite of what we discussed. Functionally there's no difference. |
|
Let's do what we discussed and have the CLI set DefaultPatchVersionForAspNetCoreAll2_1. The hard-coded value for 2.1.1 here will behave as a fallback for now. I can clean it up when dotnet/cli#9374 goes in. |
|
Thanks for helping out with this! |
Addresses: https://github.com/aspnet/websdk/issues/344.
WIP: need a 2.1.1 build of aspnetcore to fix tests.