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

render_as_json, making sorting fields optional #119

Merged
merged 9 commits into from
Dec 6, 2018
Merged

render_as_json, making sorting fields optional #119

merged 9 commits into from
Dec 6, 2018

Conversation

ritikesh
Copy link
Collaborator

  1. Added a render_as_json API - When you want a JSONified hash - useful when you want to modify the generated hash but require it to be JSONified
  2. Added a configuration option to make sorting optional - for those who don't want to sort the fields by name but want it in the order it was defined. Bonus - adds a perf benefit.
  3. Update readme with latest updates and some missing public API specs like render_as_hash, exclude..

@ritikesh
Copy link
Collaborator Author

@philipqnguyen @mcclayton
Hi guys, any updates here?

@mcclayton
Copy link
Contributor

Thanks so much for your valuable contribution @ritikesh. Apologies for the delay in reviewing, been a little slow with the holidays 🦃

@ritikesh
Copy link
Collaborator Author

Thanks so much for your valuable contribution @ritikesh. Apologies for the delay in reviewing, been a little slow with the holidays 🦃

No worries, hope had a good time there. 😄

@ritikesh
Copy link
Collaborator Author

@mcclayton I've made the changes as suggested. Please let me know if you need anything else.

@ritikesh
Copy link
Collaborator Author

ritikesh commented Dec 6, 2018

Hi guys, is there anything pending on this? @philipqnguyen @mcclayton

Copy link
Contributor

@AllPurposeName AllPurposeName 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, thanks for making those changes.

@AllPurposeName AllPurposeName merged commit 6941c49 into procore-oss:master Dec 6, 2018
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