Skip to content

streamable: Improve dataclass_from_dict validation, add more tests #10652

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jun 14, 2022
Merged

streamable: Improve dataclass_from_dict validation, add more tests #10652

merged 11 commits into from
Jun 14, 2022

Conversation

xdustinface
Copy link
Contributor

@xdustinface xdustinface commented Mar 10, 2022 •
edited
Loading

Improves the validation in all convert_<whatever> methods and in dataclass_from_dict ands adds test for all failure cases.

Based on:

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2022

This pull request fixes 1 alert when merging cb86485b6012cbcbe05bf3e97f5acc05d3d704a8 into 0e29dbc - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@xdustinface xdustinface force-pushed the pr-streamable-improve-dataclass-from-dict branch from cb86485 to edb1657 Compare March 10, 2022 18:18
@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2022

This pull request fixes 1 alert when merging edb1657152e0610e39585238b7e42bb6b74b0843 into 0e29dbc - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@github-actions
Copy link
Contributor

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added stale-pr Flagged as stale and in need of manual review merge_conflict Branch has conflicts that prevent merge to main labels Apr 25, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@xdustinface xdustinface force-pushed the pr-streamable-improve-dataclass-from-dict branch from edb1657 to e4d392b Compare June 3, 2022 16:48
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Jun 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Jun 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@xdustinface xdustinface force-pushed the pr-streamable-improve-dataclass-from-dict branch from e4d392b to 50dd51d Compare June 4, 2022 03:40
@xdustinface xdustinface requested a review from altendky June 4, 2022 03:41
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Jun 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

@xdustinface xdustinface marked this pull request as ready for review June 4, 2022 03:41
@xdustinface xdustinface marked this pull request as draft June 4, 2022 22:18
@xdustinface xdustinface force-pushed the pr-streamable-improve-dataclass-from-dict branch from 50dd51d to d043e73 Compare June 7, 2022 22:33
@xdustinface xdustinface force-pushed the pr-streamable-improve-dataclass-from-dict branch from d043e73 to a2ded6d Compare June 7, 2022 23:37
@xdustinface xdustinface marked this pull request as ready for review June 7, 2022 23:37
@xdustinface xdustinface removed the stale-pr Flagged as stale and in need of manual review label Jun 8, 2022
@xdustinface xdustinface mentioned this pull request Jun 8, 2022
Merged
@xdustinface xdustinface mentioned this pull request Jun 10, 2022
Merged
altendky
altendky reviewed Jun 14, 2022
View reviewed changes
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

I know we chatted about this, but on the isinstance() vs. type() == maybe either make them consistent or provide an explanation.

@xdustinface
Copy link
Contributor Author

I know we chatted about this, but on the isinstance() vs. type() == maybe either make them consistent or provide an explanation.

See 1b8c9fe.

@xdustinface xdustinface added the ready_to_merge Submitter and reviewers think this is ready label Jun 14, 2022
@wallentx wallentx requested review from emlowe, trepca and wjblanke June 14, 2022 15:21
@wallentx wallentx merged commit 20f5bb2 into Chia-Network:main Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@altendky altendky altendky approved these changes

@emlowe emlowe Awaiting requested review from emlowe

@trepca trepca Awaiting requested review from trepca

@wjblanke wjblanke Awaiting requested review from wjblanke

Assignees
No one assigned
Labels
ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xdustinface @altendky @wallentx