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

[build] Ignore CA1305 in more projects #8110

Merged
merged 1 commit into from
Jun 6, 2023
Merged

Conversation

pjcollins
Copy link
Member

@pjcollins pjcollins commented Jun 5, 2023

Commit 04e4bb1 turned CA1305 into a build error for projects in the
src directory, however the .NET analyzers that detect this issue only
run by default on projects targeting .NET 5.0 or later. Sources that
target netstandard2.0 (including our build tasks) contain instances of
CA1305 that should be fixed, but these errors weren't being reported
in our regular builds.

Our nightly build managed to catch some of these issues, as this build
set an MSBuild property that would cause our sources to import the FxCop
NuGet package.

FxCop is deprecated, and it should be replaced by using the first-party
.NET analyzers. These analyzers can be enabled for projects with older
target frameworks by setting the $(EnableNETAnalyzers) property
to true. This property is now set to true for all build types (pr,
CI, nightly) and projects in xamarin-android.

After enabling .NET analyzers on projects with older target frameworks,
the following projects needed to suppress CA1305 warnings as errors to
fix the regular and nightly builds:

  • src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Xamarin.ProjectTools.csproj
  • src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj
  • src/Xamarin.Android.Tools.Aidl/Xamarin.Android.Tools.Aidl.csproj
  • src/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csproj

We should try to address the CA1305 issues in these non-test projects
in a future PR.

Commit 04e4bb1 turned `CA1305` into a build error for projects in the
`src` directory, however the .NET analyzers that detect this issue only
run by default on projects targeting .NET 5.0 or later.  Sources that
target `netstandard2.0` (including our build tasks) contain instances of
`CA1305` that should be fixed, but these errors weren't being reported
in our regular builds.

Our nightly build managed to catch _some_ of these issues, as this build
set an MSBuild property that would cause our sources to import the FxCop
NuGet package.

FxCop is deprecated, and it should be replaced by using the first-party
.NET analyzers.  These analyzers can be enabled for projects with older
target frameworks by setting the [`$(EnableNETAnalyzers)`][0] property
to `true`.  This property is now set to true for all build types (pr,
CI, nightly) and projects in xamarin-android.

After enabling .NET analyzers on projects with older target frameworks,
the following projects needed to suppress `CA1305` warnings as errors to
fix the regular and nightly builds:

 * `src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Xamarin.ProjectTools.csproj`
 * `src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj`
 * `src/Xamarin.Android.Tools.Aidl/Xamarin.Android.Tools.Aidl.csproj`
 * `src/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csproj`

We should try to address the `CA1305` issues in these non-test projects
in a future PR.

[0]: https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#enablenetanalyzers
@pjcollins
Copy link
Member Author

I filed #8109 to track future work for fixing more CA1305 instances. We will likely want to prioritize those in Xamarin.Android.Build.Tasks at least.

Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

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

Approved. Just re-running some of the failed builds. They look unrelated.

@jonpryor jonpryor merged commit b1479a4 into main Jun 6, 2023
@jonpryor jonpryor deleted the dev/pjc/nightly-ca1305 branch June 6, 2023 13:08
grendello added a commit to grendello/xamarin-android that referenced this pull request Jun 7, 2023
* main:
  [ci] Shut down dotnet processes before retrying failed unit tests. (dotnet#8107)
  Bump to dotnet/installer@18dc2cf1 8.0.100-preview.6.23305.2
  [Xamarin.Android.Build.Tasks] Ignore non-Android XML resources (dotnet#8091)
  [build] Ignore CA1305 in more projects (dotnet#8110)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants