search

JoshData / jot

JSON Operational Transformation (JOT)
355 stars 34 forks source link

jot.LIST did not serialize and deserialize properly; LISTs stay lists #8

Closed gatlin closed 8 years ago

gatlin commented 8 years ago

Child elements in the ops key were not serialized with their _type information. I added checks in toJsonableObject and opFromJsonableObject for cases where a key was named ops and satisfied Array#isArray - in those cases, the children were mapped over with the appropriate procedures.

Also in certain cases in the list compose method a non-list would be returned. If You need me to separate these into two commits or something I'm glad to but it was unintuitive behavior in practice.

JoshData commented 8 years ago

Wow thanks. Looks good. Could you just add test cases for the things you've fixed?

gatlin commented 8 years ago

I'll get on that! (Thanks for insisting on tests)

On October 11, 2016 7:12:21 PM CDT, Joshua Tauberer notifications@github.com wrote:

Wow thanks. Looks good. Could you just add test cases for the things you've fixed?

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/JoshData/jot/pull/8#issuecomment-253083551

Sent from my Android device with K-9 Mail. Please excuse my brevity.

gatlin commented 8 years ago

Where is a good place to put the tests? tests/objects makes good sense to me but I'm not picky.

On 10/11/2016 07:12 PM, Joshua Tauberer wrote:

Wow thanks. Looks good. Could you just add test cases for the things you've fixed?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JoshData/jot/pull/8#issuecomment-253083551, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMspLNZ24WC6NGUXlBO-Akll-Uwhoi5ks5qzCXlgaJpZM4KUJpN.

JoshData commented 8 years ago

The tests files are named in parallel to the main source files. So for tests about the LIST class, tests/meta.js would be best.

JoshData commented 8 years ago

(Which I realize doesn't exist yet.)

gatlin commented 8 years ago

yay a wild test appeared!

JoshData commented 8 years ago

Thanks! Merged!

gatlin commented 8 years ago

Awesome! Your project is very handy.

On October 11, 2016 8:06:29 PM CDT, Joshua Tauberer notifications@github.com wrote:

Thanks! Merged!

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/JoshData/jot/pull/8#issuecomment-253090867

Sent from my Android device with K-9 Mail. Please excuse my brevity.

JoshData commented 8 years ago

I never found a use for it myself so I am glad someone else did!