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

When using (*JsonParseNode).GetXValue methods it panics fix #143

Merged
merged 12 commits into from
Aug 14, 2024

Conversation

michaeldcanady
Copy link

@michaeldcanady michaeldcanady commented Aug 9, 2024

Added utils for converting from like types to others:
ex:

  • int8 -> float64
  • float32 -> float64
  • float64 -> int8 (as long as it doesn't exceed int8 max and/or have a decimal place)
    doesn't convert between non-like types:
    bool(true) -/-> "true"
    int8(1) -/-> bool(true)

Fixes #142

@michaeldcanady michaeldcanady requested a review from a team as a code owner August 9, 2024 12:52
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I did leave one question about the impact here.

util.go Show resolved Hide resolved
Copy link
Contributor

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

@michaeldcanady
Copy link
Author

reopening, it closed, by mistake, when I went to resolve the conflicts

baywet
baywet previously approved these changes Aug 13, 2024
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!
Can you also add an entry to the changelog please? (patch bump, changed, today's date)
@andrueastman for final review and merge/publish

@baywet baywet linked an issue Aug 13, 2024 that may be closed by this pull request
@michaeldcanady
Copy link
Author

Thank you for making the changes! Can you also add an entry to the changelog please? (patch bump, changed, today's date) @andrueastman for final review and merge/publish

Just added, let me know if any adjustments should be made to the verbiage!

Copy link
Member

@andrueastman andrueastman left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution here @michaeldcanady

@andrueastman andrueastman merged commit 332365e into microsoft:main Aug 14, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG] When using (*JsonParseNode).GetXValue methods it panics
3 participants