Skip to content
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.

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

Add constructor resolution to datacontractresolver #48

Merged
merged 3 commits into from
Nov 26, 2019

Conversation

Sikta
Copy link
Contributor

@Sikta Sikta commented Nov 22, 2019

No description provided.

Copy link
Member

@dstelljes dstelljes left a comment

Choose a reason for hiding this comment

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

a few small things, otherwise 👍

also if you want practice updating the documentation site, at some point the mapping guide will need to be updated to reflect these changes

@@ -347,6 +347,13 @@ public override TypeResolution Resolve(Type type)
/// </summary>
public class EnumResolverCase : ReflectionResolverCase
{
/// <summary>
Copy link
Member

Choose a reason for hiding this comment

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

not needed on EnumResolverCase?

@@ -445,6 +452,22 @@ public override TypeResolution Resolve(Type type)
/// </summary>
public class EnumerableResolverCase : ReflectionResolverCase
{
/// <summary>
/// The binding flags that will be used to select fields and properties.
Copy link
Member

@dstelljes dstelljes Nov 26, 2019

Choose a reason for hiding this comment

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

constructors in this case, not fields and properties

/// Creates a new object resolver case.
/// </summary>
/// <param name="memberVisibility">
/// The binding flags that will be used to select fields and properties.
Copy link
Member

@dstelljes dstelljes Nov 26, 2019

Choose a reason for hiding this comment

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

constructors in this case, not fields and properties

src/Chr.Avro/Resolution/ReflectionResolver.cs Show resolved Hide resolved
@Sikta Sikta merged commit 07086bf into development Nov 26, 2019
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.

None yet

3 participants