Skip to content
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

update the branding for 5.0.2 #45799

Merged
merged 1 commit into from
Dec 11, 2020
Merged

update the branding for 5.0.2 #45799

merged 1 commit into from
Dec 11, 2020

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented Dec 8, 2020

  • branding updates, pcks already contains the 5.0.1 in packageindex

@ericstj whats the correct entry in the package index

"5.0.0.0": "5.0.0" or "5.0.0.0":"5.0.0.2" ? what are the rules here

@ghost
Copy link

ghost commented Dec 8, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details
  • branding updates
  • need to service the extensions and pipelines package to have 5.0.0.0 assembly version for the aspnet shared framework

@ericstj whats the correct entry in the package index

"5.0.0.0": "5.0.0" or "5.0.0.0":"5.0.0.2" ? what are the rules here

Author: Anipik
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@ericstj
Copy link
Member

ericstj commented Dec 9, 2020

@ericstj whats the correct entry in the package index

"5.0.0.0": "5.0.0" or "5.0.0.0":"5.0.0.2" ? what are the rules here

These indicate which assembly version appears in which package version. Left side is assembly version, right side is package version AssemblyVersionInPackageVersion

@@ -1,7 +1,6 @@
<Project>
<Import Project="..\Directory.Build.props" />
<PropertyGroup>
<PackageVersion>5.0.1</PackageVersion>
<AssemblyVersion>5.0.0.1</AssemblyVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this showing as a diff here? We can't go backwards in AssemblyVersion. Was this shipped this way? I thought you had reverted the AssemblyVersions in the shipping package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were late in the cycle so we decided to fix this in 5.0.2

Copy link
Member

@ViktorHofer ViktorHofer Dec 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Anipik just to double check, do you intend to ship the 5.0.2 version of this package with 5.0.0.0 as the assembly version even though the 5.0.1 version of the package had an assembly version of 5.0.0.1? Wouldn't that be breaking change in various components?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that this shipped with the incremented version, we have to keep it:

// C:\Users\erics\AppData\Local\Temp\Temp1_microsoft.extensions.dependencyinjection.5.0.1.nupkg\lib\net5.0\Microsoft.Extensions.DependencyInjection.dll
// Microsoft.Extensions.DependencyInjection, Version=5.0.0.1, Culture=neutral, PublicKeyToken=adb9793829ddae60

@Anipik
Copy link
Contributor Author

Anipik commented Dec 10, 2020

These indicate which assembly version appears in which package version. Left side is assembly version, right side is package version AssemblyVersionInPackageVersion

we can have only one entry corresponding to 5.0.0.0 so should we update this to 5.0.2 or keep it to 5.0.0

@Anipik
Copy link
Contributor Author

Anipik commented Dec 10, 2020

cc @akoeplinger can u take a look at the mono failure, it seems legit. @maryamariyan it seems related to the dependencyInjection change in 5.0.1

@akoeplinger
Copy link
Member

The mono failure is addressed with #45887 so you can ignore it.

@maryamariyan
Copy link
Member

maryamariyan commented Dec 10, 2020

@Anipik noticed failure below from the CI logs: (link)

Microsoft.Extensions.DependencyInjection.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: Microsoft.Extensions.DependencyInjection.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  Microsoft.Extensions.DependencyInjection.Tests (found 99 of 429 test cases)
  Starting:    Microsoft.Extensions.DependencyInjection.Tests (parallel test collections = on, max threads = 2)
    Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactoryTest.CreateCallSite_Throws_IfClosedTypeDoesNotSatisfyAbstractClassGenericConstraint [FAIL]
      System.NullReferenceException : Object reference not set to an instance of an object
      Stack Trace:
        /_/src/mono/netcore/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs(384,0): at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Finished:    Microsoft.Extensions.DependencyInjection.Tests
=== TEST EXECUTION SUMMARY ===
   Microsoft.Extensions.DependencyInjection.Tests  Total: 214, Errors: 0, Failed: 1, Skipped: 0, Time: 0.434s

building your branch locally now to check for repro.

@maryamariyan
Copy link
Member

maryamariyan commented Dec 10, 2020

building your branch locally now to check for repro.

I'm not running on mono, so couldn't reproduce the failure locally.

@safern
Copy link
Member

safern commented Dec 10, 2020

I'm not running on mono, so couldn't reproduce the failure locally.

You could run on mono by building with

build.cmd mono+libs -rc Release

Then run the tests as usual but pass down /p:RuntimeFlavor=mono.

@Anipik Anipik merged commit 5d131e0 into dotnet:release/5.0 Dec 11, 2020
@Anipik Anipik deleted the branding502 branch December 11, 2020 01:01
@ghost ghost locked as resolved and limited conversation to collaborators Jan 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants