search

ardite / ardite-core

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

Schema DSL #23

Closed calebmer closed 8 years ago

calebmer commented 8 years ago

This is the start of a refactor of how core types in Ardite schema code are defined. Why? For extensibility and backwards compatibility purposes. By hiding the SchemaType enum we have the freedom to change it and its values whenever we want. And by exposing the values it holds by functions, we can add new functions whenever we want. We can also cast types using Into where appropriate.

@svmnotn: Take a look, tell me what you think, and then I'll implement.

calebmer commented 8 years ago

Ok, take a look now and tell me what you think.

svmnotn commented 8 years ago

This is much better. It would be nice if you could clean up the commit history a bit. But other than that I think it is ready for impls

calebmer commented 8 years ago

Ok, fixed the MongoDB errors (of course CI doesn't know…). There is still no solution for testing equality, I don't think that's a requirement for getting this merged, what do you think?

svmnotn commented 8 years ago

Where would you be testing for eq?

calebmer commented 8 years ago

Here to test if our deserialization is working as expected, and here to test if the Schema::get method is returning the right things. Especially the first tests are a big deal.

calebmer commented 8 years ago

Almost ready to merge! Got the schema to implement PartialEq using the Debug trait. Now all that I want to do is decide how we are going to document/crate/pass around Schemas. Give me a bit and I'll ping you once its ready for review.

calebmer commented 8 years ago

Actually, nevermind, I'm happy with the current interface. Give it a review and lets merge it!

svmnotn commented 8 years ago

Other than the test mod refactor, the rest looks mergeable

calebmer commented 8 years ago

I was planning on doing the test mod refactor in another PR.

svmnotn commented 8 years ago

Then this is :+1: for merge