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

Support optional serialize and unserialize #36

Merged
merged 3 commits into from
Aug 31, 2017
Merged

Support optional serialize and unserialize #36

merged 3 commits into from
Aug 31, 2017

Conversation

Runrioter
Copy link
Contributor

When I am restructuring some existing code that wrote by Java and PHP, I have a problem. Data in store was serialize or unserialize in Java/PHP Object serialization, so I want to make this repo support serialize/unserialize.

Please review it.

@coveralls
Copy link

coveralls commented Aug 30, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling fc77292 on Runrioter:master into 7e4940a on koajs:master.

@DaAwesomeP
Copy link
Collaborator

@Runrioter Thank you! I really like this idea. Could you please add tests for serialize and unserialize? It can be something really simple (ex. adding some characters to the send of JSON.stringify() in serialize so that it will fail JSON.parse() unless it goes through unserialize).

@coveralls
Copy link

coveralls commented Aug 31, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 5d79370 on Runrioter:master into 7e4940a on koajs:master.

@DaAwesomeP DaAwesomeP merged commit da0721e into koajs:master Aug 31, 2017
@DaAwesomeP
Copy link
Collaborator

Thanks!

@Runrioter
Copy link
Contributor Author

@DaAwesomeP my pleasure

@Runrioter
Copy link
Contributor Author

@DaAwesomeP Can you release a patch version? I will use it in production.

@DaAwesomeP
Copy link
Collaborator

@Runrioter I'm going to wait a day or so to hear from #34. In the meantime, you can specify your fork as a source to NPM.

@DaAwesomeP
Copy link
Collaborator

@Runrioter I published v3.1.1.

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.

3 participants