-
Notifications
You must be signed in to change notification settings - Fork 467
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
Runtime90357 6012 #6885
Runtime90357 6012 #6885
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6885 +/- ##
=======================================
Coverage 96.39% 96.39%
=======================================
Files 1403 1403
Lines 330977 331069 +92
Branches 10890 10894 +4
=======================================
+ Hits 319056 319150 +94
+ Misses 9187 9186 -1
+ Partials 2734 2733 -1 |
...Tests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs
Show resolved
Hide resolved
Please don't resolve this issue if we merged this PR. We need to have some changes in the runtime repo to remove suppressing CC @buyaa-n |
Updated comment to not fix that issue |
Not sure why the builds fail. It has probably to do with the new description added. |
src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx
Outdated
Show resolved
Hide resolved
...yzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs
Outdated
Show resolved
Hide resolved
Looks it needs to run |
...yzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs
Outdated
Show resolved
Hide resolved
...Tests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs
Show resolved
Hide resolved
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.
Once CI is fully green and @buyaa-n has signed off on the PR, I approve of this for RC2.
@carlossanlop, when it reaches that point, can you please shepherd this through Tactics and into the RC2 builds?
Yes, no problem. Assuming this PR is done before Sept 18th (RC2 snap day), your approval @jeffhandley is sufficient (no Tactics involvement needed). Note: the |
@buyaa-n That did seem to do the trick, but the only change is removing a line for another analyzer from RulesMissingDocumentation.md, which seems completely unrelated to this PR. |
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.
Than you @manfred-brands LGTM
@carlossanlop for analyzers repo we used to merge PRs into release branch first then it flows automatically to main, is that same this year or now it's changed? CC @mavasani |
Merged, @manfred-brands we do squash merge so if you planning to put up a port to 8.0 please cherry pick squashed commit. Or I can do that later |
/backport to release/8.0.1xx |
Started backporting to release/8.0.1xx: https://github.com/dotnet/roslyn-analyzers/actions/runs/5980358523 |
Never mind, thanks to #6892 now we have a bot for backporting. |
Fixes #6012
See also dotnet/runtime#90357
Prevents IndexOutOfRangeException, but also checks formats of non-formatting methods.