Move Roslyn.sln to newer analyzers (with Dispose rules disabled) #25891
Conversation
mavasani
added
the
PR For Personal Review Only
build/Targets/Packages.props
Outdated
| <MicrosoftNetCoreILAsmVersion>2.0.0</MicrosoftNetCoreILAsmVersion> | ||
| <MicrosoftNETCoreCompilersVersion>2.8.0-beta2-62712-07</MicrosoftNETCoreCompilersVersion> | ||
| <MicrosoftNETCoreCompilersVersion>2.7.0-beta3-62720-08</MicrosoftNETCoreCompilersVersion> |
There was a problem hiding this comment.
Why are we downgrading these packages?
There was a problem hiding this comment.
@jmarolf I have tried lot of different versions of this package, including 2.8.X version, on a separate PR (#25179), but none of them seem to work. I tried the latest 2.7 version as Jason recommended trying it out, but that did not work either. I have reverted the downgrade to see if it works now. At this point, I am happy to put any version of this package that unblocks this PR.
There was a problem hiding this comment.
I would like to understand the dependencies here. @jaredpar what is MicrosoftNETCoreCompilersVersion and why is it different from MicrosoftNetCompilersVersion?
mavasani
removed
the
PR For Personal Review Only
|
@dotnet/roslyn-compiler Please review. This PR is needed to unblock @DustinCampbell's MSBuild work: #21670 |
|
@dotnet-bot retest windows_debug_unit32_prtest please // Reason: Flaky test failure tracked with #25898 |
|
@dotnet-bot retest windows_debug_unit32_prtest please |
|
@dotnet/roslyn-infrastructure Integration tests are not running, is this known? |
|
CLA appears to be stuck again too. 😦 |
|
I guess this is your run? https://ci.dot.net/job/dotnet_roslyn/job/master/job/windows_debug_vs-integration_prtest/247/ Maybe we're hitting some generic token limit which is why all sorts of stuff isn't being reported? |
| @@ -11,6 +11,7 @@ | |||
| <DefaultLanguage>en-US</DefaultLanguage> | |||
| <TargetFrameworks>$(RoslynPortableTargetFrameworks46)</TargetFrameworks> | |||
| <DisableImplicitFrameworkReferences>false</DisableImplicitFrameworkReferences> | |||
| <NoWarn>$(NoWarn);CA1819</NoWarn> <!-- CA1819 (Properties should not return arrays) disabled as it is very common across this project. --> | |||
There was a problem hiding this comment.
Nit: Please put the comment above property. That's the general style we use in build files.
💭 I disabled that test in 3c001e3 |
CLA status check also went flaky around this time. Together that does sound like a GitHub token limit. |
| @@ -5,6 +5,8 @@ | |||
| using Microsoft.Build.Framework; | |||
| using Microsoft.Build.Utilities; | |||
|
|
|||
| #pragma warning disable CA1819 // CA1819: Properties should not return arrays | |||
There was a problem hiding this comment.
Not sure if we should have some special case for build tasks -- this is a very common pattern there.
|
|
Ask Mode template not completed
Customer scenario
What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)
Bugs this fixes
(either VSO or GitHub links)
Workarounds, if any
Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW
Risk
This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code
Performance impact
(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")
Is this a regression from a previous update?
Root cause analysis
How did we miss it? What tests are we adding to guard against it in the future?
How was the bug found?
(E.g. customer reported it vs. ad hoc testing)
Test documentation updated?
If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.