Skip to content

Fix MatchInfoContext Clone implementation - #5121

Merged
Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
dee-see:master
Oct 18, 2017
Merged

Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
dee-see:master

Conversation

@dee-see

Copy link
Copy Markdown
Contributor

Fix #3994

The old implementation always set the new object's properties to null.

The original issue mentioned that the other Clone implementation in MatchString.cs had the same bug, but in fact it's correct because of the use of MemberwiseClone.

I didn't much unit testing of C# code going on in the project (or maybe I missed it?). Should I add a test and if so, where?

The old implementation always set the new object's properties to null

@lzybkr Jason Shirk (lzybkr) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code change is fine.

A test would be nice, ideally failing if a new property is introduced but not cloned. That might be hard, so just checking the number of properties and a good comment might be sufficient.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 81c46c6 into PowerShell:master Oct 18, 2017
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
The old implementation always set the new object's properties to null.
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