Skip to content

Conversation

kripken
Copy link
Member

@kripken kripken commented Feb 1, 2024

Our barebones JSON impl will be used more in a future PR. This adds some
simple functionality to it.

It will be tested in the upcoming PR, which I can combine this into if that
would be preferable?

@kripken kripken requested a review from tlively February 1, 2024 00:51
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Is it worth having unit tests for this?


namespace json {

void Value::stringify(std::ostream& os, bool pretty) {
Copy link
Member

Choose a reason for hiding this comment

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

This was previously declared but never defined or used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it just had a declaration for years... this finally starts to implement it 😄

@kripken
Copy link
Member Author

kripken commented Feb 1, 2024

Added a test.

@kripken kripken enabled auto-merge (squash) February 1, 2024 19:17
@kripken kripken merged commit 5526027 into WebAssembly:main Feb 1, 2024
@kripken kripken deleted the json branch February 1, 2024 19:52
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants