Skip to content

Skip type collection if property has MessagePackFormatterAttribute#1462

Merged
AArnott merged 1 commit intoMessagePack-CSharp:masterfrom
gllebede-havok:user/gllebede/skip_properties_with_MessagePackFormatterAttribute
Jul 23, 2022
Merged

Skip type collection if property has MessagePackFormatterAttribute#1462
AArnott merged 1 commit intoMessagePack-CSharp:masterfrom
gllebede-havok:user/gllebede/skip_properties_with_MessagePackFormatterAttribute

Conversation

@gllebede-havok
Copy link
Copy Markdown
Contributor

The MessagePackFormatterAttribute defines a custom formatter so there is no reason to handle it automatically.

@AArnott AArnott requested a review from neuecc June 29, 2022 13:24
Copy link
Copy Markdown
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Looks safe enough. I gather the intended behavioral change is to respect the attribute where we didn't before? Or is this really just a perf optimization?

@gllebede-havok
Copy link
Copy Markdown
Contributor Author

I gather the intended behavioral change is to respect the attribute where we didn't before? Or is this really just a perf optimization?

It's a fix for a bug. A property can define a MessagePackFormatter for a property so, from my understanding, it should use the type in the property. Instead, it tries to generate a formatter for property return type which may not be possible.

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