AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
684 stars 217 forks source link

Enable PublicApiAnalyzers #3055

Closed westin-m closed 1 month ago

westin-m commented 1 month ago

See: https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md

Include internals for assemblies that are marked with InternalsVisibleTo

JoshLozensky commented 1 month ago

Do we need to be enabling the analyzers on our tests? Their methods aren't packed into our libraries.

JoshLozensky commented 1 month ago

Is there a process for moving things from unshipped to shipped?

ciaozhang commented 1 month ago

Is there a process for moving things from unshipped to shipped?

It is a step for release process. We have guidance in MISE: https://identitydivision.visualstudio.com/DevEx/_git/MISE?path=/internaldocs/Public_API_Analyzer.md&_a=preview

keegan-caruso commented 1 month ago

mark-shipped should look like this: https://github.com/dotnet/aspnetcore/blob/2287c782562158ebe86b438ab9792db244764b48/eng/scripts/mark-shipped.ps1

JoshLozensky commented 1 month ago

It is a step for release process. We have guidance in MISE:

What is the impact if we get this wrong when updating? As a partially manual process we will probably get it wrong at some point. Like if we revert a change but forget to move it back to unreleased, or miss adding changes to the framework file, miss a forwarded type, etc...

westin-m commented 1 month ago

What is the impact if we get this wrong when updating?

If a member is part of the declared API and we have to revert/remove/etc. but fail to make the appropriate change to the .txt file(s), the analyzer will see the diff and produce RS0017, which should enforce moving it to "Unshipped".
Additionally, if it's in a project with internal tracking, there would be an error for not declaring a member that we perhaps move back to Internal.

westin-m commented 1 month ago

Do we need to be enabling the analyzers on our tests? Their methods aren't packed into our libraries.

No, those files slipped past me after being generated automatically.