Skip to content

Adapt CtorParameterAndPropertySetterExists test for v2.3 behavior - #1156

Merged
AArnott merged 3 commits into
MessagePack-CSharp:developfrom
AArnott:docCtorPlusSetter_v2.3
Jan 2, 2021
Merged

Adapt CtorParameterAndPropertySetterExists test for v2.3 behavior#1156
AArnott merged 3 commits into
MessagePack-CSharp:developfrom
AArnott:docCtorPlusSetter_v2.3

Conversation

@AArnott

@AArnott AArnott commented Dec 21, 2020

Copy link
Copy Markdown
Collaborator

See also #1155

CC: @pCYSl5EDgo who introduced this breaking change. Was the breaking change necessary for what you were delivering (#1095)?

@AArnott AArnott added this to the v2.3 milestone Dec 21, 2020
@AArnott
AArnott requested a review from neuecc December 21, 2020 19:49
@AArnott AArnott self-assigned this Dec 21, 2020
@pCYSl5EDgo

Copy link
Copy Markdown
Contributor

Oh, I was not aware of that this breaking change caused this situation!
Thank you for pointing out!

I introduced this breaking change in order not to set parameters twice(constructor and setter).

@AArnott

AArnott commented Dec 23, 2020

Copy link
Copy Markdown
Collaborator Author

IMO setting them twice is wasteful and the new behavior is better. But I am less sure it is worth the behavioral change. Perhaps we can keep it for now and revert this part of your change if folks complain.

@pCYSl5EDgo

Copy link
Copy Markdown
Contributor

I agree with the reverting of this behaviour when folks complain it.

@AArnott
AArnott merged commit 4ca7e42 into MessagePack-CSharp:develop Jan 2, 2021
@AArnott
AArnott deleted the docCtorPlusSetter_v2.3 branch January 2, 2021 14:42
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.

2 participants