KEMBAR78
Make the interface `IDeepCloneable` internal by daxian-dbw · Pull Request #25552 · PowerShell/PowerShell · GitHub
Skip to content

Make the interface IDeepCloneable internal #25552

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 2, 2025
Merged

Make the interface IDeepCloneable internal #25552

merged 2 commits into from
Jun 2, 2025

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented May 13, 2025 •
edited
Loading

PR Summary

This PR is a follow-up of the #25497.
The deep clone support in Microsoft.Management.UI.Internal should be made an implementation detail, so make the IDeepCloneable interface internal instead.

PR Checklist

@daxian-dbw daxian-dbw assigned daxian-dbw and unassigned daxian-dbw May 13, 2025
@daxian-dbw daxian-dbw added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log and removed CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels May 13, 2025
@daxian-dbw
Copy link
Member Author

@mawosoft please let me know if you any concerns.

@mawosoft
Copy link
Contributor

@daxian-dbw I don't have any concerns, it's just odd to pick one among many. None of these classes should probably be public.

Also, nit: If you make it internal, you can remove the suppression attribute.

@daxian-dbw
Copy link
Member Author

The suppression attribute is removed. The main reason to make the IDeepCloneable interface internal is to rule out the possibility that someone may implement this interface and feed such an object to Out-GridView to break the clone in any unexpected way.

@daxian-dbw daxian-dbw requested a review from TravisEz13 May 13, 2025 19:51
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label May 21, 2025
SeeminglyScience
SeeminglyScience approved these changes Jun 2, 2025
View reviewed changes
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM

May not strictly be needed, but also doesn't need to be public so if we have the option to minimize confusion we might as well.

@daxian-dbw daxian-dbw removed the Review - Needed The PR is being reviewed label Jun 2, 2025
@daxian-dbw daxian-dbw merged commit 4e03546 into PowerShell:master Jun 2, 2025
51 of 53 checks passed
@daxian-dbw daxian-dbw deleted the make-it-private branch June 2, 2025 22:18
@microsoft-github-policy-service
Copy link
Contributor

microsoft-github-policy-service bot commented Jun 2, 2025 •
edited by unfurl-links bot
Loading

📣 Hey @@daxian-dbw, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@SeeminglyScience SeeminglyScience mentioned this pull request Jun 3, 2025
Merged
9 tasks
pwshBot pushed a commit to pwshBot/PowerShell that referenced this pull request Jun 3, 2025
@daxian-dbw @pwshBot
Cherry-pick PR PowerShell#25552 with conflicts for manual resolution
090b52c
@pwshBot pwshBot mentioned this pull request Jun 3, 2025
Closed
9 tasks
pwshBot pushed a commit to pwshBot/PowerShell that referenced this pull request Aug 6, 2025
@daxian-dbw @pwshBot
Make the interface IDeepCloneable internal to minimize confusion (P…
c6fa3c1 
…owerShell#25552)
@pwshBot pwshBot mentioned this pull request Aug 6, 2025
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov iSazonov iSazonov approved these changes

@SeeminglyScience SeeminglyScience SeeminglyScience approved these changes

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13

Assignees

No one assigned

Labels

BackPort-7.5.x-Done CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

PowerShell Team Reviews/Investigations
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@daxian-dbw @mawosoft @iSazonov @SeeminglyScience @TravisEz13