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 initial README #70

Merged
merged 1 commit into from
Apr 19, 2024
Merged

docs: Add initial README #70

merged 1 commit into from
Apr 19, 2024

Conversation

maxveldink
Copy link
Owner

No description provided.

Copy link

@minukmarkchoi minukmarkchoi left a comment

Choose a reason for hiding this comment

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

hey, thanks for this detailed file - it definitely fills in the gaps and provides details into what this repo is about! Looks promising, can't wait to do a deep dive and put it to use!

Left a question about the error message that is returned when parsing fails, but it is a small subjective nitpick.

```ruby
# Unparsable JSON
result = json_serializer.deserialize('{"name""Max","age":29}')
result.error # == Typed::ParseError: json could not be parsed. Check for typos.

Choose a reason for hiding this comment

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

subjective nitpick: should acronyms like json be capitalized? JSON vs json?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I tend to capitalize it when referring to it in docs or comments, and use snake_case conventionally for variables. I think the error message probably should be capped, but would say that's out of scope for this PR.

@maxveldink maxveldink merged commit 1187fdd into main Apr 19, 2024
5 checks passed
@maxveldink maxveldink deleted the mv/update-readme branch April 19, 2024 12:41
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.

2 participants