-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove AD0001 NoWarn from Microsoft.Extensions.Logging.Abstractions project #91222
Conversation
Tagging subscribers to this area: @dotnet/area-extensions-logging |
@buyaa-n I assume we'll need to backport this to rc2, right? CC @jeffhandley |
The analyzer fix backported to rc2, not sure if we need to backport this change |
I prefer to backport this one too. Note, 8.0 is LTS and expects some servicing fixes on such projects. Keep disabling such warning is risky when we fix anything in that project. At the same time, the fix here is safe as we are not touching any product code. |
Sounds good to me, I guess we can run the porting bot after merge |
Just ensure the analyzer fix is already flow to the 8/0 release branch. otherwise, this will fail there. |
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.
Thanks for seeing this through. I was on the fence regarding the net8 backport, but @tarekgh's reasoning sounds good to me. Once the backport PR is ready, let me know and I'll approve it.
It is not flowed yet #91154, not sure if the PR is blocked, CC @carlossanlop |
No, not blocked, but the CI keeps getting restarted because more commits get added. I can merge it now. |
/backport to release/8.0 |
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6004858438 |
The analyzer bug that was causing #90357 fixed and the changes flowed to the runtime
Fixes #90357