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

expanded documentation for using Oj to replace JSON #714

Merged
merged 2 commits into from
Oct 10, 2021

Conversation

jjb
Copy link
Contributor

@jjb jjb commented Oct 7, 2021

No description provided.

@jjb
Copy link
Contributor Author

jjb commented Oct 7, 2021

@ohler55 I threw this together. give me any and all feedback, i won't be offended if you don't like my copy. I took the bold move of making a prominent new section in the Readme. Im thinking this is useful, when I was trying to figure this out I was surprised to not find a quick guide in the docs or blog posts.

let me know what you think!

question: is Oj::Rails.mimic_JSON different from Oj.mimic_JSON?

irb(main):012:0> Oj::Rails.method(:mimic_JSON) == Oj.method(:mimic_JSON)
=> false

README.md Outdated
@@ -32,6 +32,15 @@ puts "Same? #{h == h2}"
# true
```

## Universally and seamlessly replacing slow JSON methods with faster Oj equivalents
Copy link
Owner

Choose a reason for hiding this comment

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

Oj is used often with Rails but this section places Oj use in rails before even the installation. A better place would be in the Rails.md file or maybe somewhere in the Further Reading section area.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@ohler55
Copy link
Owner

ohler55 commented Oct 7, 2021

I think you have highlighted the need for a useful addition to the readme. There are a few different ways to categorize Oj features but I think most would break down the use as either rails, json gem replacement, or native Oj use. Native Oj use could be broken down further but if a section on the different uses is added it should cover all three albeit only briefly along the lines of what you started with.

@jjb jjb force-pushed the doc-additions branch 4 times, most recently from 66d1ca8 to f42c06d Compare October 10, 2021 19:49
@jjb
Copy link
Contributor Author

jjb commented Oct 10, 2021

thanks for the feedback!

i refined my approach and the copy, see the new small sections below Installation. (i had previously put my additions before installation because that's where the existing Using section is)

rational for keeping something in the main PR is i'm guessing the portion of people coming to the readme to find this specific information is very large.

let me know if you still want changes!

when it looks good i'll squish the commits

@ohler55
Copy link
Owner

ohler55 commented Oct 10, 2021

Looks good.

@ohler55 ohler55 merged commit 1d219ea into ohler55:develop Oct 10, 2021
@jjb jjb deleted the doc-additions branch October 10, 2021 22:29
@jjb
Copy link
Contributor Author

jjb commented Oct 10, 2021

Cool!

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.

None yet

2 participants