Re-enable api check

[JunTaoLuo]

#2350 Unblocked by aspnet/BuildTools@d944172

[JunTaoLuo]

API check ran but it found a breaking change: fdb4184#diff-dfd6e5bf3bc39e44af0761394afeb8ecR16 and c3ba875#diff-dfd6e5bf3bc39e44af0761394afeb8ecR18. Should I add an exception or do we need to add an overload for the constructor?
cc @halter73

[davidfowl]

That package was never released to nuget.org so it's fine.

[JunTaoLuo]

@davidfowl in that case I think we should remove the baseline.json file right?

[davidfowl]

@davidfowl in that case I think we should remove the baseline.json file right?

Yep!

[JunTaoLuo]

@dotnet-bot test this please

[JunTaoLuo]

@dotnet-bot test Windows Release x64 Build

[JunTaoLuo]

API breaking change in abdcb47

[davidfowl]

@JunTaoLuo I think you should skip libuv for API check and file a blocking bug in preivew2, then we need to discuss how we treat pubternal API changes that are in public APIs (specifically this public API).

/cc @DamianEdwards @Eilon

[Eilon]

@davidfowl it isn't clear to me what the breaking change is here by looking at the linked diffs. Changes in public APIs that use pubternals are not considered breaking changes by us, so those would be OK.

[JunTaoLuo]

@Eilon Actually it's not a pubternal breaking change. The method signature of the constructor of LibuvTransportFactory changed and it's a public type. I believe the discussion is that this type should have remained pubternal.

[JunTaoLuo]

Filed follow up issue at: #2424. I want to unblock the API checks for the other packages ASAP.

[halter73]

IMHO the issue is that LibuvTransportFactory never should have been fully public. Instead it should be pubternal.

The only public way to use or configure libuv should be through the UseLibuv extension methods.

[JunTaoLuo]

@dotnet-bot test Ubuntu 16.04 Release Build

[ryanbrandenburg]

Just a nitpick on the ApiCheck technicalities, if we haven't shipped a package yet what we're doing for other packages is to have a baseline file for them, but have it be empty. Not really sure I have enough context to weigh in on the Libuv stuff though.

[halter73]

@ryanbrandenburg Are you saying that src/Kestrel.Transport.Sockets/baseline.netcore.json should be regenerated from the release/2.1 branch since it hasn't been shipped yet? If so, do we just regenerate the baseline if we change public API prior to 2.1 RTM?

[ryanbrandenburg]

@halter73 no, I'm saying that if we haven't shipped Kestrel.Transport.Sockets yet we should have a blank file at src/Kestrel.Transport.Sockets/baseline.netcore.json. It indicates to future travelers "this space intentionally left blank", rather than that we just forgot to generate it.

[halter73]

LGTM. I can submit another PR to pubternalize LibuvTransportFactory, add an API check exception and reenable the check for the libuv transport.

[halter73]

@ryanbrandenburg @pranavkm Will either of you be mad if I cherry-pick b90766d into Kestrel's release/2.1 branch?

[JunTaoLuo]

Absolutely furious, but I'll get over it. /s

[ryanbrandenburg]

@halter73 not as long as you then merge the resulting commit back into dev.