Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix CA1052 for public API#15775

Merged
adityapatwardhan merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:CA1052-p2xtqqczze/PowerShell-PowerShell:CA1052-p2Copy head branch name to clipboard
Jul 28, 2021
Merged

Fix CA1052 for public API#15775
adityapatwardhan merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:CA1052-p2xtqqczze/PowerShell-PowerShell:CA1052-p2Copy head branch name to clipboard

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Jul 14, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

xtqqczze commented Jul 14, 2021

Copy link
Copy Markdown
Contributor Author

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

SteveL-MSFT commented Jul 21, 2021

Copy link
Copy Markdown
Member

@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

adityapatwardhan commented Jul 28, 2021

Copy link
Copy Markdown
Member

@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

ghost commented Aug 23, 2021

Copy link
Copy Markdown

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

Handy links:

@ghost

ghost commented Sep 28, 2021

Copy link
Copy Markdown

🎉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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.