Skip to content

Fix CA1052 for public API#15775

Merged
adityapatwardhan merged 3 commits intoPowerShell:masterfrom
xtqqczze:CA1052-p2
Jul 28, 2021
Merged

Fix CA1052 for public API#15775
adityapatwardhan merged 3 commits intoPowerShell:masterfrom
xtqqczze:CA1052-p2

Conversation

@xtqqczze
Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze commented Jul 14, 2021 •
edited
Loading

Fix CA1052: Static holder types should be Static or NotInheritable for public API.

  • Add static modifier to static holder types
  • Remove empty constructor (not valid anymore in static class

Possible breaking change: Unlikely Grey Area

Instantiating a class containing only static members has no obvious utility.

@xtqqczze
Copy link
Copy Markdown
Contributor Author

xtqqczze commented Jul 14, 2021

CodeFactor "complex method" issues are false positives, see #15721.

@xtqqczze xtqqczze marked this pull request as ready for review July 15, 2021 11:54
@xtqqczze xtqqczze changed the title Fix CA1052 Fix CA1052 for public API Jul 15, 2021
@iSazonov iSazonov added Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jul 21, 2021
@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jul 21, 2021
@SteveL-MSFT
Copy link
Copy Markdown
Member

SteveL-MSFT commented Jul 21, 2021

@PowerShell/powershell-committee reviewed this, we don't believe there's any real case where someone would be broken by this as the classes only have static members. We approve of this change.

@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jul 22, 2021
@adityapatwardhan adityapatwardhan merged commit 055b7f3 into PowerShell:master Jul 28, 2021
@adityapatwardhan adityapatwardhan added this to the 7.2.0-preview.9 milestone Jul 28, 2021
@adityapatwardhan
Copy link
Copy Markdown
Member

adityapatwardhan commented Jul 28, 2021

@xtqqczze Thank you for your contribution

@xtqqczze xtqqczze deleted the CA1052-p2 branch July 28, 2021 21:21
@xtqqczze xtqqczze mentioned this pull request Jul 29, 2021
Merged
@ghost
Copy link
Copy Markdown

ghost commented Aug 23, 2021

🎉v7.2.0-preview.9 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link
Copy Markdown

ghost commented Sep 28, 2021

🎉v7.2.0-preview.10 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13

Assignees

@adityapatwardhan adityapatwardhan

Labels

Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

7.2.0-preview.9

Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze @SteveL-MSFT @adityapatwardhan @iSazonov