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 Json Serialization #2794

Closed
wants to merge 1 commit into from
Closed

Add Json Serialization #2794

wants to merge 1 commit into from

Conversation

erikzhang
Copy link
Member

No description provided.

@superboyiii
Copy link
Member

@erikzhang Can you add some unit tests first. This is a large modification.

@cschuchardt88
Copy link
Member

Why we keep reinventing the wheel here? We should be using NewtonSoft.Json or something similar.

@Jim8y
Copy link
Contributor

Jim8y commented Jan 4, 2024

Why we keep reinventing the wheel here? We should be using NewtonSoft.Json or something similar.

For making the whole process controllable and determinastic.

@vncoelho
Copy link
Member

vncoelho commented Jan 4, 2024

As well as performance.
We did some experiments in the past.
Maybe nowadays it is faster but not sure.

@cschuchardt88
Copy link
Member

But we are using a stone wheel here on a spot car for instance.

@vncoelho we don't use huge json. So benchmarks will not show much change. But as for the useability and flexibility it's 1000 times better than what we have, and plus we can tie into other systems easily. It will help fix #3037 out of the box and also has what this PR is reinventing. So all the problems that you will eventually find or run into, are already done with any good json library on nuget like NewtonSoft.Json (157,196,338 downloads; that should tell you something)

@vncoelho
Copy link
Member

vncoelho commented Jan 4, 2024

I am not against it.
Perhaps it is worth give a try.

@Jim8y Jim8y added the Need Active Pr will be closed after one week if no new activity. label Feb 12, 2024
@Jim8y
Copy link
Contributor

Jim8y commented Feb 18, 2024

Close as inactive

@Jim8y Jim8y closed this Feb 18, 2024
@shargon shargon deleted the json-serialization branch February 18, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need Active Pr will be closed after one week if no new activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants