Skip to content

Change Tools TFMs to net7.0 #6088

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

Merged
merged 9 commits into from
Aug 31, 2022
Merged

Change Tools TFMs to net7.0 #6088

merged 9 commits into from
Aug 31, 2022

Conversation

lbussell
Copy link
Contributor

Roslyn analyzers needs a target framework of .NET 7.0 for source build to build without prebuilts. There are some projects in the Tools/ directory of roslyn-analyzers that still target net6.0.

@lbussell lbussell requested a review from a team as a code owner July 29, 2022 21:56
@lbussell
Copy link
Contributor Author

I've been trying to get this to work but keep running into the error:

CS8032: Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. The system cannot find the file specified.

I'm not sure where we are finding that version of Microsoft.CodeAnalysis, I can't find any references to it in this repo.

@Youssef1313
Copy link
Member

@lbussell I think this is the version used by dotnet/runtime. The compiler used in the repository is likely a version lower than 4.2.0.

Copy link
Contributor

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

LGTM

For the build failure due to new IDExxxx code style diagnostics, feel free to set the diagnostic to an IDE suggestion or silent in the .editorconfig file at the repo root and please file a separate tracking issue to fix the violations and revert the .editorconfig change.

For example:

dotnet_diagnostic.IDE0150.severity = silent

@lbussell
Copy link
Contributor Author

@mavasani, I filed an issue like you suggested: #6128

I don't know what to do about the RS2003 errors:

error RS2003: (NETCORE_ENGINEERING_TELEMETRY=Build) Rule 'RS0001' was shipped in analyzer release '3.3.0', but is no longer a supported diagnostic for any analyzer. Add an entry for this rule in a 'Removed Rules' table to unshipped file.

I would try to edit the markdown files but they say not to edit them manually.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Can this wait until #6117 is merged?

.editorconfig Outdated
Comment on lines 20 to 23
# IDE0190: Null check can be simplified
# IDE0200: Lambda expression can be removed
dotnet_diagnostic.IDE0200.severity = silent
dotnet_diagnostic.IDE0190.severity = silent
Copy link
Member

Choose a reason for hiding this comment

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

Once #6117 is merged, these should be removed.

@@ -39,7 +39,7 @@
<!-- Use the correct compiler version -->
<UsingToolMicrosoftNetCompilers>true</UsingToolMicrosoftNetCompilers>
<!-- Dependencies from https://github.com/dotnet/roslyn -->
<MicrosoftNETCoreCompilersPackageVersion>4.0.0-4.final</MicrosoftNETCoreCompilersPackageVersion>
<MicrosoftNETCoreCompilersPackageVersion>4.2.0</MicrosoftNETCoreCompilersPackageVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Once #6117 is merged, it will produce a conflict here. The version from #6117 should be used.

@Youssef1313
Copy link
Member

I don't know what to do about the RS2003 errors:

They'll go away once #6117 is merged.

@lbussell
Copy link
Contributor Author

Can this wait until #6117 is merged?

It can wait as long as we can get both changes into 7.0 RC2.

@mavasani
Copy link
Contributor

#6117 has been merged

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #6088 (61b7381) into main (dc672ef) will decrease coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6088      +/-   ##
==========================================
- Coverage   96.05%   96.05%   -0.01%     
==========================================
  Files        1343     1343              
  Lines      309713   309713              
  Branches     9956     9956              
==========================================
- Hits       297490   297489       -1     
  Misses       9822     9822              
- Partials     2401     2402       +1     

@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net7.0</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

Consider updating this line:

4. Generate notes: Switch to the output directory, say `artifacts\bin\ReleaseNotesUtil\Debug\net6.0` and execute `GenDiffNotes.cmd` to generate release notes. Example command line for v2.9.4 to v2.9.5: `GenDiffNotes.cmd C:\scratch nuget.org 2.9.4 2.9.5`.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

LGTM with a minor documentation update.

I think it should be okay to update the documentation in a separate follow-up PR if you want to avoid waiting for CI again for this PR.

@lbussell
Copy link
Contributor Author

CI was broken due to dotnet/arcade#10248 anyways so I updated it.

@lbussell
Copy link
Contributor Author

@mavasani it looks like all the checks passed. Or did some of them not run?

@mavasani
Copy link
Contributor

@mavasani it looks like all the checks passed. Or did some of them not run?

Yes we should be good to merge after an approval

@mavasani mavasani merged commit 6a57872 into dotnet:main Aug 31, 2022
@github-actions github-actions bot added this to the vNext milestone Aug 31, 2022
@jmarolf jmarolf modified the milestones: vNext, .NET 7.x Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants