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: Add documentation on AST and Cursor #1098

Merged

Conversation

BalduinLandolt
Copy link
Contributor

I really like the latest additions to the decoding documentation.
However, I feel that the concepts of the AST and Cursor aren't really introduced/explained to the extent that the examples would make sense for someone who isn't already familiar with the concepts.

I jotted down a little something. But I'm also happy to make changes or see this changed by the maintainers.

Also, since the decoding.md page is getting pretty long, it might be worth changing some of the overall structure of the documentation, maybe with an additional level of page nesting.
That's of course not mine to decide, but I'd be happy to contribute to it, if I can.

@BalduinLandolt BalduinLandolt requested a review from a team as a code owner April 26, 2024 20:06
docs/decoding.md Outdated

To get the AST representation of a JSON string, use the `fromJson[Json]` method.

```scala
Copy link
Member

Choose a reason for hiding this comment

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

I prefer using compile-safe code snippets to keep the documents up-to-date using mdoc modifiers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right... it even says so in the docs contribution guidelines. Sorry, missed that - will update my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Note that the CI now fails because of an non-up-tp-date README file, which seems to relate to 61b7634

Copy link
Member

Choose a reason for hiding this comment

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

Note that the CI now fails because of an non-up-tp-date README file, which seems to relate to 61b7634

@BalduinLandolt
Please update the PR from upstream series/2.x branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khajavi thanks for updating it! Should be good to merge now?

@BalduinLandolt BalduinLandolt force-pushed the wip/add-ast-and-cursor-documentation branch from 165c818 to 764325a Compare May 13, 2024 04:54
@BalduinLandolt
Copy link
Contributor Author

@khajavi ping 🙂
could this be merged?

@fsvehla fsvehla merged commit 6e7079c into zio:series/2.x Jun 3, 2024
28 checks passed
@BalduinLandolt BalduinLandolt deleted the wip/add-ast-and-cursor-documentation branch June 4, 2024 22:14
ThijsBroersen pushed a commit to ThijsBroersen/zio-json that referenced this pull request Jun 13, 2024
Co-authored-by: Milad Khajavi <khajavi@gmail.com>
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.

4 participants