Identity: Reenable API check

[HaoK]

disabled to unblock build to get Auth 2.0 build unblocked

[HaoK]

Revert 2b343726897b2cece524537d56aea4c80ac265bd and update baseline

[Eilon]

@HaoK / @ajcvickers - needed for preview2, or revisit in 2.1.0-rc1? @ryanbrandenburg - does your work with ApiCheck cover everything here anyway?

[HaoK]

@ryanbrandenburg already updated the baselines, so I think we just need to turn EnableApiCheck on again...I'll do that today and hopefully nothing fails :)

[HaoK]

@ryanbrandenburg @javiercn what do we want to do about the ui package, should we wait until closer to rtm to generate baselines + enable api check for this since its new in 2.1?

[Eilon]

I think the new packages, such as the UI ones, should get empty baselines.

[Eilon]

(That way we know it needs to be checked, but there will be no breaking changes detected, because everything is an 'add'.)

[HaoK]

So getting one breaking change with constants moving from protected to public...

 public class Microsoft.AspNetCore.Identity.UserManager<T0> : System.IDisposable where T0 : class
      protected const System.String ChangePhoneNumberTokenPurpose = "ChangePhoneNumber"
      protected const System.String ConfirmEmailTokenPurpose = "EmailConfirmation"
      protected const System.String ResetPasswordTokenPurpose = "ResetPassword"

Following is the list of exclusions that either need to be added to the list of breaking changes, or the breaking changes themselves need to be reverted:
[
{
"TypeId": "public class Microsoft.AspNetCore.Identity.UserManager : System.IDisposable where T0 : class",
"MemberId": "protected const System.String ChangePhoneNumberTokenPurpose = \"ChangePhoneNumber\"",
"Kind": "Removal"
},
{
"TypeId": "public class Microsoft.AspNetCore.Identity.UserManager : System.IDisposable where T0 : class",
"MemberId": "protected const System.String ConfirmEmailTokenPurpose = \"EmailConfirmation\"",
"Kind": "Removal"
},
{
"TypeId": "public class Microsoft.AspNetCore.Identity.UserManager : System.IDisposable where T0 : class",
"MemberId": "protected const System.String ResetPasswordTokenPurpose = \"ResetPassword\"",
"Kind": "Removal"
}
]

[HaoK]

They were protected in 2.0, and were made public in 2.1...

Current: https://github.com/aspnet/Identity/blob/dev/src/Core/UserManager.cs
2.0: https://github.com/aspnet/Identity/blob/release/2.0/src/Microsoft.Extensions.Identity.Core/UserManager.cs

[HaoK]

@Eilon don't we still have to do an update to add the final 2.1 shape? I mean I guess it doesn't hurt to turn it on with an empty baseline, but that's basically just having it no-op instead of waiting to turning it on when we actually want it to do something? I'll file a separate issue for tracking the 2.1 final baseline update to UI since we need that regardless

[Eilon]

Re: The existing breaking changes. I would vote to avoid the breaking change, because it doesn't seem worthwhile. But please work with the mgr/PM to resolve it one way or another.

We will update to the final 2.1 shape after we ship 2.1. I don't think we need https://github.com/aspnet/Identity/issues/1712 to track it because it will be a bulk update of all baselines. (That's why we want the empty baselines - so we know what to update.)

[HaoK]

Ok cool, bulk update will be nice, I'll add empty baselines as part of the PR reenabling the checks

[HaoK]

Is making a constant public from protected really considered a breaking change? We increased the visibility here...

[Eilon]

Ah, you know what, maybe it isn't.

[Eilon]

https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/breaking-change-rules.md#members

Members
✓ Allowed

...

Increasing the visibility of a member that is not virtual

[Eilon]

So these are fine - just add the breaking change exceptions and then I think log a bug in the BuildTools repo to improve ApiCheck's behavior.

[HaoK]

Cool, let me just double check on why we did this, I vaguely remember there might have been a customer ask for this, I want to make sure it wasn't accidental before updating the baselines

[HaoK]

Ok updating the baselines, we did this as part of https://github.com/aspnet/Identity/issues/1325

[javiercn]

@Eilon I had an idea for this actually.
For new projects we can have a property
<ApiCheckVersionThreshold>
If the assembly we are producing has <= than the version, we don't run API check.
Once the Version goes higher, we run it and fail/produce warnings because there's no baseline.

I don't like having empty baselines because it silently disables the tool.

[Eilon]

I thought empty baselines are fine because the tool will run, but will always pass. Then when we finish 2.1.0 and start on 2.2.0 we will update all the existing baselines (even if empty) to 2.1.0-RTM.

[javiercn]

I thought empty baselines are fine because the tool will run, but will always pass

That's why it silently disables the tool. If we update the baselines when we release 2.1 then sure, but we need to do that.

[Eilon]

Yes we have to do that anyway for every repo.

[ajcvickers]

Empty baselines seems like the way to go to me as well. If we don't update baselines after releases we will have more problems than just this.

[HaoK]

c7711c2515c22be25f615eca60ab3348c9ffbd47