Skip to content
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

Update Versions.props to enable releasing NetAnalyzers 5.0.1 package #4488

Merged
merged 2 commits into from
Dec 3, 2020

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Dec 3, 2020

This PR enables seamless migration from 3.x FxCop analyzers packages published out of master to a stable NetAnalyzers package by following steps at https://docs.microsoft.com/visualstudio/code-quality/migrate-from-fxcop-analyzers-to-net-analyzers

Will revert this PR as soon as we get a signed build candidate for publishing 5.0.1 package for latest bug fixes. We need to finalize our long term story on which branch do we target bug fixes to existing analyzers versus add new .NET vNext analyzers.

@mavasani mavasani requested a review from a team as a code owner December 3, 2020 00:17
Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Although I'm not entirely sure I understand the reason why we can't keep releasing 5.x version packages

Copy link
Contributor

@jmarolf jmarolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, ping me when this goes in so I can setup all the flows for this. At this time I assume we want to create a new branch for 6.0 and have master flow into it?

@mavasani
Copy link
Contributor Author

mavasani commented Dec 3, 2020

I'm not entirely sure I understand the reason why we can't keep releasing 5.x version packages

That is primarily to ensure how AnalysisLevel property gets translated to the correct global analyzer config file, so that if user sets AnalysisLevel to 5 we can skip analyzers introduced in version 6. The mapping to global config relies on analyzer package version:

<Target Name=""AddGlobalAnalyzerConfigForPackage_{trimmedPackageName}"" BeforeTargets=""CoreCompile"" Condition=""'$(SkipGlobalAnalyzerConfigForPackage)' != 'true'"">
<!-- PropertyGroup to compute global analyzer config file to be used -->
<PropertyGroup>{propertyStringForSettingDefaultPropertyValue}
<!-- Set the default analysis mode, if not set by the user -->
<_GlobalAnalyzerConfigAnalysisMode>$(AnalysisMode)</_GlobalAnalyzerConfigAnalysisMode>
<_GlobalAnalyzerConfigAnalysisMode Condition=""'$(_GlobalAnalyzerConfigAnalysisMode)' == ''"">{nameof(AnalysisMode.Default)}</_GlobalAnalyzerConfigAnalysisMode>
<!-- GlobalAnalyzerConfig file name based on user specified package version '{packageVersionPropName}', if any. We replace '.' with '_' to map the version string to file name suffix. -->
<_GlobalAnalyzerConfigFileName Condition=""'$({packageVersionPropName})' != ''"">AnalysisLevel_$({packageVersionPropName}.Replace(""."",""_""))_$(_GlobalAnalyzerConfigAnalysisMode).editorconfig</_GlobalAnalyzerConfigFileName>
<_GlobalAnalyzerConfigDir Condition=""'$(_GlobalAnalyzerConfigDir)' == ''"">$(MSBuildThisFileDirectory)config</_GlobalAnalyzerConfigDir>
<_GlobalAnalyzerConfigFile Condition=""'$(_GlobalAnalyzerConfigFileName)' != ''"">$(_GlobalAnalyzerConfigDir)\$(_GlobalAnalyzerConfigFileName)</_GlobalAnalyzerConfigFile>
</PropertyGroup>
<ItemGroup Condition=""Exists('$(_GlobalAnalyzerConfigFile)')"">
<EditorConfigFiles Include=""$(_GlobalAnalyzerConfigFile)"" />
</ItemGroup>
</Target>
";
static string GetPropertyStringForSettingDefaultPropertyValue(string packageName, string packageVersionPropName)
{
if (packageName == NetAnalyzersPackageName)
{
return $@"
<!-- Default '{packageVersionPropName}' to 'EffectiveAnalysisLevel' with trimmed trailing '.0' -->
<{packageVersionPropName} Condition=""'$({packageVersionPropName})' == '' and $(EffectiveAnalysisLevel) != ''"">$([System.Text.RegularExpressions.Regex]::Replace($(EffectiveAnalysisLevel), '(.0)*$', ''))</{packageVersionPropName}>
";

@mavasani
Copy link
Contributor Author

mavasani commented Dec 3, 2020

At this time I assume we want to create a new branch for 6.0 and have master flow into it?

Yes, that seems fine. Once the branch is created, I'll request all open PRs for new analyzers to retarget to release/6.0.xx.

@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #4488 (704496d) into master (32d97c9) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4488      +/-   ##
==========================================
- Coverage   95.84%   95.83%   -0.01%     
==========================================
  Files        1175     1175              
  Lines      268047   268047              
  Branches    16174    16174              
==========================================
- Hits       256905   256885      -20     
- Misses       9068     9073       +5     
- Partials     2074     2089      +15     

@mavasani mavasani merged commit 3b7bb4a into master Dec 3, 2020
@mavasani mavasani deleted the netanalyzers-5-0-1 branch December 3, 2020 02:05
@jmarolf
Copy link
Contributor

jmarolf commented Dec 3, 2020

alrignt the branch is created and I've updates the version number: https://github.com/dotnet/roslyn-analyzers/tree/release/6.0.1xx-preview1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants