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

(docs) Cookbooks "Serialise JSON Data using ppx_yojson_conv" and "Deserialise JSON Data using ppx_yojson_conv" #2415

Merged
merged 16 commits into from
Jun 24, 2024

Conversation

gpopides
Copy link
Contributor

@gpopides gpopides commented May 7, 2024

I have added 2 new cookbook files for json serialization and deserialization. I believe this task is pretty common and would help people trying to learn the language.

I'm not an ocaml expert so i first looked myself in the cookbooks to see if there is one.

I also haven't found a PR for this, so i thought about sharing what i had to use for my needs.

I also have followed the idea of the yaml cookbook.

Any feedback would be welcome

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Thanks @gpopides! ✨ Indeed, JSON is so common it is very useful to have this!

data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
@christinerose
Copy link
Collaborator

Let me know when this is ready for my review. The ize will need to be changed to ise for OCaml.org, but I'll do all that along with the other line editing. Just ping me when you're ready, and I'll ignore the notifications before then.

@gpopides
Copy link
Contributor Author

gpopides commented May 8, 2024

Let me know when this is ready for my review. The ize will need to be changed to ise for OCaml.org, but I'll do all that along with the other line editing. Just ping me when you're ready, and I'll ignore the notifications before then.

Sure, i will ping you a bit later when i will be able to adjust it based on the comments. Thanks!

gpopides and others added 6 commits May 8, 2024 18:18
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
Co-authored-by: sabine <6594573+sabine@users.noreply.github.com>
@gpopides
Copy link
Contributor Author

gpopides commented May 8, 2024

I have applied all of the suggestions (thanks for that) so the comments are more understandable now. Let me know if we need to adjust anything else

@sabine sabine added Cookbook needs review We would love to have your help reviewing this! labels May 9, 2024
Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

@gpopides: Can you break lines to make them shorter? That would improve the rendering with the current cookbook UX

Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Comment blocks must be a single comment, sorry for that

data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
Co-authored-by: Cuihtlauac Alvarado <cuihtlauac@users.noreply.github.com>
Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

Format identifiers in comments

data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
data/cookbook/serialise-to-json/00-serialize-yojson.ml Outdated Show resolved Hide resolved
@sabine sabine changed the title Add cookbooks for JSON serialization and deserialization (docs) Cookbooks JSON serialization and deserialization Jun 20, 2024
@sabine sabine changed the title (docs) Cookbooks JSON serialization and deserialization (docs) Cookbooks Jun 20, 2024
@sabine sabine changed the title (docs) Cookbooks (docs) Cookbooks "Serialise JSON Data using ppx_yojson_conv" and "Deserialise JSON Data using ppx_yojson_conv" Jun 20, 2024
Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Thanks @gpopides!

Merging after @christinerose line editing.

Copy link
Collaborator

@christinerose christinerose left a comment

Choose a reason for hiding this comment

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

Looks great!

@sabine sabine merged commit 77fa6a4 into ocaml:main Jun 24, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cookbook needs review We would love to have your help reviewing this!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants