-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Add support for packaging analyzers into ref-pack #33977
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
Conversation
Co-authored-by: Doug Bunting <[email protected]>
Separately, will the CI actually validate the new code path prior to dotnet/runtime#54950 getting in and a new runtime build flowing into this repo❔ |
Not at the moment (it's a feature that lets the work remain decoupled 😄). Do you have a place where you test that FrameworkList task? |
Yes, please see https://github.com/dotnet/aspnetcore/blob/main/src/Framework/test/TargetingPackTests.cs#L297-L393 |
Those tests appear to be testing the product content, not running the task itself with test-inputs, is that correct? In either case I should update those tests so that they don't break when the actual transport package arrives. Is there any other place where you'd want this to be tested in lieu of the transport package? |
Yes. Our
No that seems sufficient. I like the thought of not only being ready in advance of whatever Maestro++ PR brings in your dotnet/runtime#54950 work but also doing more than just ignoring the new files listed in the framework list. |
I don't see any failures clearly tied to your changes but would rather merge on green. Most test failures look like transient problems but the |
Separately I saw you merged dotnet/runtime#54950 and fix should have affected Microsoft.AspNetCore.Internal.Transport and Microsoft.NETCore.App.Ref v6.0.0-preview.7.21351.5 (from build https://dev.azure.com/dnceng/internal/_build/results?buildId=1215933&view=results) or newer. However, while System.Text.Json.SourceGeneration.dll was in Microsoft.NETCore.App.Ref, Microsoft.Extensions.Logging.Generators.dll was not in Microsoft.AspNetCore.Internal.Transport. The transport package still lacks an analyzers/ folder. Is something more needed before that shows up❔ |
Good call. Let me fix that. My test used a manual mockup on the version ASP.NET is using today. |
/azp run aspnetcore-ci |
Azure Pipelines successfully started running 1 pipeline(s). |
Seems to be passing now @dougbu. Also fix should be in here: dotnet/runtime#55042 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have done this hours ago
Related: dotnet/runtime#54687
Spec: https://github.com/dotnet/designs/blob/main/accepted/2021/InboxSourceGenerators.md
We'll start flowing source-generators through the transport package in dotnet/runtime#54950. This will pick them up and include them in the ASP.NET refpack.
The framework list change is a port of dotnet/arcade#7561.