Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.6k
Remove date number from dev build version#1835
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9e10218642d09b0aac40a0e70f8f785b157466589235babe36c19aa84525288File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,10 +6,12 @@ | ||
| <MajorVersion>5</MajorVersion> | ||
| <MinorVersion>0</MinorVersion> | ||
| <PatchVersion>0</PatchVersion> | ||
| <!-- Always use shipping version instead of dummy version --> | ||
| <DotNetUseShippingVersions>true</DotNetUseShippingVersions> | ||
| <PreReleaseVersionLabel>alpha</PreReleaseVersionLabel> | ||
| <PreReleaseVersionIteration>1</PreReleaseVersionIteration> | ||
| <!-- Set assembly version to align with major and minor version, | ||
| as for the patches and revisions should be manually updated per assembly if it is serviced. --> | ||
| <AssemblyVersion>$(MajorVersion).$(MinorVersion).0.0</AssemblyVersion> | ||
| <!-- Enable to remove prerelease label. --> | ||
| <StabilizePackageVersion Condition="'$(StabilizePackageVersion)' == ''">false</StabilizePackageVersion> | ||
| <DotNetFinalVersionKind Condition="'$(StabilizePackageVersion)' == 'true'">release</DotNetFinalVersionKind> | ||
| @@ -43,7 +45,7 @@ | ||
| <MicrosoftDotNetRemoteExecutorVersion>5.0.0-beta.20071.3</MicrosoftDotNetRemoteExecutorVersion> | ||
| <MicrosoftDotNetVersionToolsTasksVersion>5.0.0-beta.20071.3</MicrosoftDotNetVersionToolsTasksVersion> | ||
| <!-- Installer dependencies --> | ||
| <MicrosoftNETCoreAppVersion>5.0.0-alpha.1.19562.8</MicrosoftNETCoreAppVersion> | ||
| <MicrosoftNETCoreAppVersion>5.0.0-alpha.1.20071.1</MicrosoftNETCoreAppVersion> | ||
MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm concerned that this version is being used... the installer tests need to be running on the current build's outputs or else they aren't covering anything. 😕 Can you link the issue you mentioned you'd file for this? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised the installer tests use this property, I thought this property was used by package testing only. I mentioned it here: Could you elaborate how the installer tests use this instead of the recently built product? MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, I'm not familiar with it and I'm surprised by it when you mentioned you had to change this to make the installer tests work. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't mean installer tests, I mean libraries all configuration package tests... we run some tests on our oob packages to make sure closure is complete and that we don't ship duplicate types and that all supported rids for those packages publish correctly, etc. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah. This should be moved out from Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, that was a result from the repo merge as in corefx it was "core-setup dependencies" maybe when we consolidated it was just a find and replace from "core-setup" to "installer". Will remove. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, what it means is that those dependencies as built by the installer. If you look the file, those comments split the sections on where are those packages coming from, for example look at runtime-assets dependencies which comes from https://github.com/dotnet/runtime-assets or arcade dependencies. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, that makes sense. Sorry for the confusion on this, no need to change it. | ||
| <MicrosoftNETCoreDotNetHostVersion>5.0.0-alpha.1.19563.6</MicrosoftNETCoreDotNetHostVersion> | ||
| <MicrosoftNETCoreDotNetHostPolicyVersion>5.0.0-alpha.1.19563.6</MicrosoftNETCoreDotNetHostPolicyVersion> | ||
| <MicrosoftExtensionsDependencyModelVersion>2.1.0</MicrosoftExtensionsDependencyModelVersion> | ||
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.
Can we leave this set to true when we're in an Official build? So that package versions are unique?
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.
Not necessary. https://github.com/dotnet/arcade/blob/bca68c2df2c6230389f10def3880a61db266c6c1/src/Microsoft.DotNet.Arcade.Sdk/tools/Version.BeforeCommonTargets.targets#L39
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.
Awesome, arcade is ahead of me 😄