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

[release/8.0.1xx] Remove CA2229 Implement serialization constructors #6915

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 6, 2023

Backport of #6911 to release/8.0.1xx

/cc @buyaa-n

Customer Impact

Hidden rule CA2229: Implement serialization constructors, can be enabled specifically by ID or can be enabled as part of bulk configuration. Starting in .NET 8 SYSLIB0051: Legacy serialization support APIs are obsolete generates warning at compile time for the serialization constructors (and also for other BF related APIs) therefore conflicts with CA2229.

As Binary formatters are deprecated in .NET Core CA2229 is also out dated and can be removed.

Testing

NA, removing the analyzer entirely

Risk

Very low:

  • CA2229 could be useful for .NET framework users that are referencing NetAnalyzers manually. If they still need the analyzer, they could keep using the older versions.
  • CA2229 also could be useful .NET core users that opt in using BF. Though BF is going to be removed from .NET 9 anyways.

For both cases even if they missed adding the constructor a basic app testing would uncover such a bug, so it is less risky.

@github-actions github-actions bot requested a review from a team as a code owner September 6, 2023 21:12
@buyaa-n buyaa-n added this to the 8.0.1xx milestone Sep 6, 2023
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #6915 (8b83ecc) into release/8.0.1xx (546d9ed) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                Coverage Diff                @@
##           release/8.0.1xx    #6915    +/-   ##
=================================================
  Coverage            96.40%   96.40%            
=================================================
  Files                 1403     1396     -7     
  Lines               331069   330339   -730     
  Branches             10894    10882    -12     
=================================================
- Hits                319153   318452   -701     
+ Misses                9184     9160    -24     
+ Partials              2732     2727     -5     

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

This is approved for .NET 8 RC2 since it is related to the BinaryFormatter obsoletion strategy and this analyzer was found to contradict the guidance we now provide.

@buyaa-n buyaa-n merged commit 4488b8a into release/8.0.1xx Sep 7, 2023
11 checks passed
@buyaa-n buyaa-n deleted the backport/pr-6911-to-release/8.0.1xx branch September 7, 2023 16:40
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