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

fix: json decode bug #955

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

asamaayako
Copy link
Contributor

@asamaayako asamaayako commented Aug 29, 2024

Description

This PR fixes #816 , and it a retry PR.

Summary

This PR is try again. I'm having a conflict, and retrying met a can't pass banchmark😥. Maybe I need help to Pass test.
Define a function for processing json_string to PyObject

PR Checklist

Please ensure that:

  • The PR contains a descriptive title
  • The PR contains a descriptive summary of the changes
  • You build and test your changes before submitting a PR.
  • You have added relevant documentation
  • You have added relevant tests. We prefer integration tests wherever possible

Pre-Commit Instructions:

Copy link

vercel bot commented Aug 29, 2024

@asamaayako is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

@asamaayako asamaayako changed the title fix:json decode fix: json decode bug Aug 29, 2024
Copy link

codspeed-hq bot commented Aug 29, 2024

CodSpeed Performance Report

Merging #955 will not alter performance

Comparing asamaayako:fix/json_decode (1f5465e) with main (12d8c0b)

Summary

✅ 114 untouched benchmarks

@sansyrox
Copy link
Member

Hey @asamaayako 👋

Could you explain the confilct you are facing? I will be happy to help :D

@asamaayako
Copy link
Contributor Author

asamaayako commented Aug 30, 2024

Hey @asamaayako 👋

Could you explain the confilct you are facing? I will be happy to help :D

Hey @sansyrox Previously encountered a conflict, merge conflict (failed to pass test after merging the main branch into this branch). I think at the time the main branch had code that couldn't pass test. So I force push rolled back the merge.
Now it ok :D. This PR is ready to merge I think.

@sansyrox
Copy link
Member

sansyrox commented Oct 6, 2024

Hey @VishnuSanal ,

Could you review this PR?

@sansyrox
Copy link
Member

sansyrox commented Oct 6, 2024

Hey @asamaayako 👋
Could you explain the confilct you are facing? I will be happy to help :D

Hey @sansyrox Previously encountered a conflict, merge conflict (failed to pass test after merging the main branch into this branch). I think at the time the main branch had code that couldn't pass test. So I force push rolled back the merge. Now it ok :D. This PR is ready to merge I think.

@asamaayako ,
thanks for the PR. Apologies for the super late acknowledgment. Having a look now

Copy link
Contributor

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

Incorrect Data Types in JSON Serialization from Python Client to Server
3 participants