Skip to content

Conversation

@buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Aug 28, 2023

The analyzer bug that was causing #90357 fixed and the changes flowed to the runtime

Fixes #90357

@buyaa-n buyaa-n requested review from ericstj and tarekgh August 28, 2023 17:25
@ghost ghost assigned buyaa-n Aug 28, 2023
@ghost
Copy link

ghost commented Aug 28, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

The analyzer bug that was causing #90357 fixed and the changes flowed to the runtime

Fixes #90357

Author: buyaa-n
Assignees: -
Labels:

area-Extensions-Logging

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Aug 28, 2023

@buyaa-n I assume we'll need to backport this to rc2, right?

CC @jeffhandley

@tarekgh tarekgh added this to the 8.0.0 milestone Aug 28, 2023
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Aug 28, 2023

I assume we'll need to backport this to rc2, right?

The analyzer fix backported to rc2, not sure if we need to backport this change

@tarekgh
Copy link
Member

tarekgh commented Aug 28, 2023

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.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Aug 28, 2023

Sounds good to me, I guess we can run the porting bot after merge

@tarekgh
Copy link
Member

tarekgh commented Aug 28, 2023

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.

Copy link
Member

@jeffhandley jeffhandley left a 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.

@buyaa-n buyaa-n merged commit 2ccef72 into dotnet:main Aug 28, 2023
@buyaa-n buyaa-n deleted the remove_nowarn branch August 28, 2023 20:11
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Aug 28, 2023

Just ensure the analyzer fix is already flow to the 8/0 release branch. otherwise, this will fail there.

It is not flowed yet #91154, not sure if the PR is blocked, CC @carlossanlop

@carlossanlop
Copy link
Contributor

No, not blocked, but the CI keeps getting restarted because more commits get added. I can merge it now.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Aug 28, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

@ghost ghost locked as resolved and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProvideCorrectArgumentsToFormattingMethodsAnalyzer throws IndexOutOfRangeException when building Microsoft.Extensions.Logging.Abstractions

4 participants