Skip to content

Correct analysis#2961

Merged
jeremydmiller merged 7 commits into
JasperFx:mainfrom
dmytro-pryvedeniuk:correct-analysis
May 31, 2026
Merged

Correct analysis#2961
jeremydmiller merged 7 commits into
JasperFx:mainfrom
dmytro-pryvedeniuk:correct-analysis

Conversation

@dmytro-pryvedeniuk
Copy link
Copy Markdown
Contributor

@dmytro-pryvedeniuk dmytro-pryvedeniuk commented May 29, 2026

This PR addresses the following issues with code analysis

  • Microsoft.CodeAnalysis.NetAnalyzers is used with condition having a typo that resolves to false. It means that SDK version is used instead. This reference is removed to avoid confusion.
  • In Analysis.Build.props AnalysisModeReliability is set to true. It's not a valid value, valid ones are listed here - https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#analysismode. Invalid value is ignored. Removed to avoid confusion.
  • Analysis.Build.props is not referenced by all projects. Only some of them reference it directly in csproj. It means for many projects warnings are skipped. - Imported Analysis.Build.props in Directory.Build.prop.
  • New warnings discovered are addressed - either fixed or disabled with pragma.

@dmytro-pryvedeniuk dmytro-pryvedeniuk marked this pull request as draft May 29, 2026 11:36
@jeremydmiller
Copy link
Copy Markdown
Member

@dmytro-pryvedeniuk Hey man, I appreciate all this kind of thing, but if the analysis stuff causes friction, I will rip it out later

@dmytro-pryvedeniuk
Copy link
Copy Markdown
Contributor Author

dmytro-pryvedeniuk commented May 29, 2026

@jeremydmiller I don't add new analyzers, just let the existing ones to analyze all projects. Consistency matters. If with "friction" you mean false positives it's inevitable with any code analysis I am afraid.

@dmytro-pryvedeniuk dmytro-pryvedeniuk marked this pull request as ready for review May 30, 2026 08:14
@jeremydmiller
Copy link
Copy Markdown
Member

"Consistency matters." -- some things matter, other things are noise and friction. I'll try it for awhile.

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.

2 participants