Closed chancefeick closed 8 years ago
👍
We should probably add a required ruby version to the gemspec.
This generally looks good to me. Total nit but in the future you might want to do something like this in two pieces:
PR 1) functional changes PR 2) formatting changes (hashrockets, etc)
Just to make review a bit more manageable.
Also we should make sure the full integration suite passes (not run by Travis) before this is merged, lmk if you need any help to run this.
@nextmat added required version in 33c00151775cf438148505110e7c61b729e424ed and refactored SmartJSON w/ tests in 58bf7ae957de241793175288e2ecc64927da22bc, PTAL.
:+1: on breaking it out into functional/style changes in the future to prevent larger diffs.
RE: integration tests, they appear to have already been failing on master, unrelated to recent changes. Will get those passing in another PR and merge into 2.0 branch.
Remove support for ruby 1.8. Introduce the following changes:
2.0 roadmap: #110
@nextmat lmk if any other improvements/oversights come to mind