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

3049 Use poco based parsers #3073

Draft
wants to merge 8 commits into
base: develop-6.0
Choose a base branch
from

Conversation

ewoutkramer
Copy link
Member

Description

Describe the changes in this PR.

Related issues

Closes|Fixes|Resolves [issue #].

Testing

Describe how this change was tested.

FirelyTeam Checklist

  • Update the title of the PR to be succinct and less than 50 characters
  • Mark the PR with the label breaking change when this PR introduces breaking changes

@ewoutkramer ewoutkramer changed the base branch from develop to develop-6.0 March 8, 2025 06:34
@@ -0,0 +1,197 @@
/*
Copy link
Member Author

@ewoutkramer ewoutkramer Mar 8, 2025

Choose a reason for hiding this comment

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

Reviewer: this file was moved - so nothing conceptually new going on here.

buildPocoBuilderSettings(settings));


private static PocoBuilderSettings buildPocoBuilderSettings(ParserSettings ps) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

Code moved old BaseParser, which has been deleted. This was the only place it is still used.

public async Tasks.Task<T> ParseAsync<T>(JsonReader reader) where T : Base
=> (T)await ParseAsync(reader, typeof(T)).ConfigureAwait(false);

/// <inheritdoc cref="ParseAsync(string, Type)" />
Copy link
Member Author

Choose a reason for hiding this comment

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

This stuff has been moved to (legacy) extension methods you have already seen above.

Copy link
Member Author

@ewoutkramer ewoutkramer left a comment

Choose a reason for hiding this comment

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

Reviewed it for snafus, added come helpful comments for reviewer (I hope). No formal PR review.

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.

1 participant