Enable AOT compatibility for all libraries#4625
Conversation
ghost
assigned
src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/Workarounds.cs
Outdated
Show resolved
Hide resolved
- Enable configuration binder source generator - The only library that can't use the ConfigBinder SG is the HeaderParsing library. - Blocked by dotnet/runtime#94547
eerhardt
force-pushed
the
EnableConfigBinderSG
branch
from
23b1714 to
701ffec
Compare
eerhardt
changed the title
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=470141&view=codecoverage-tab |
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=470292&view=codecoverage-tab |
|
@eerhardt I suggest reducing the threshold in the csproj. The coverage report is showing 100%, but it's likely bugged (we likely need to use the newer PublishCodeCoverageResults task, but it's broken too). |
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator> |
There was a problem hiding this comment.
Is this something we could configure at the global level as opposed to doing it in each individual project?
There was a problem hiding this comment.
I wouldn't recommend it for now. This source generator uses the experimental "Interceptors" feature in C# 12. That's why it isn't enabled by default everywhere. Plus we are only using it in 10 of the ~35 projects. So I don't think it makes sense to turn it on for all. Just like we don't turn on the Logging and Metrics source generators globally.
I'm assuming it is this change that is confusing/breaking it:
I'll change MinCodeCoverage to 99 for this assembly. |
|
FYI - @ericstj @eiriktsarpalis @tarekgh - This is changing dotnet/extensions to use the configuration binder source generator in 9.0. No new issues were found. The only outstanding issues that I know about are:
|
|
@joperezr @RussKie @sebastienros - any thoughts on this PR? |
RussKie
left a comment
RussKie
left a comment
There was a problem hiding this comment.
LGTM (with my minimal knowledge of the area)
|
I'm happy to merge it if you think it's ready. |
I'm going to run CI on it one more time just to make sure no breaks came in the meantime. Once that passes, I believe this can be merged. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Enable AOT compatibility for all libraries and fix AOT warnings.
When we get a build of .NET 8 with all the source generator bugs fixed, I will port this to
main.Microsoft Reviewers: Open in CodeFlow