search

memcachier / memjs

A memcache client for node using the binary protocol and SASL authentication
MIT License
197 stars 52 forks source link

Feature/serializer #129

Closed vanatd closed 4 years ago

vanatd commented 4 years ago

Hello @alevy, due to discussion in this pr i made changes in another branch, 'cause we are using my fork.

So I implement seriliazer object which can do stuff what we need.

But i'd like to discuss implementation with you before adding new tests.

Please take a look on it and let me know what you think about it.

vanatd commented 4 years ago

@alevy Hello, that's cool thanks! I have some ideas how to simplify it.

vanatd commented 4 years ago

@alevy i made changes, when you will have some time please take a look on it.

saschat commented 4 years ago

@denis-vanat Looks much better. Can you explain the rationale behind adding the opcode to the (de)serialize functions?

vanatd commented 4 years ago

@saschat By adding opcode to (de)serialize we can decide what to do for each case. ie we can ignore (de)serialize values for append/prepend methods or implement it in another way.

vanatd commented 4 years ago

@saschat @alevy any thoughts?

saschat commented 4 years ago

@alevy Merge and publish?