search

ardite / ardite-core

Core library for Ardite services.
MIT License
3 stars 1 forks source link

Refactor: serde #38

Closed calebmer closed 8 years ago

calebmer commented 8 years ago

I'm not actually ready to merge this yet.

calebmer commented 8 years ago

Ok, review this now. There are some rough edges that I want to get your feedback on cough de.rs cough. Good news though, no more build script!

svmnotn commented 8 years ago

couch ?

calebmer commented 8 years ago

Fixed

svmnotn commented 8 years ago

de needs more tests, and I'm worried about the 'static bound on the Schema objects.

calebmer commented 8 years ago

Did you read the stackoverflow question I sent you a while back?

calebmer commented 8 years ago

And a big part of des tests is in definition.rs. I guess it makes sense to move it back to de.rs…

calebmer commented 8 years ago

Added more tests, and after reading the SO answer, are you still worried about 'static? If you need more assurance, valico is doing a very similar thing to our Schema type with their Validator type. And look here + here.

svmnotn commented 8 years ago

Could use some more tests but it's fine for now.

calebmer commented 8 years ago

So merge?