search

ardite / ardite-core

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

Custom types! #19

Closed calebmer closed 8 years ago

calebmer commented 8 years ago

This PR is not yet ready to be merged. A quick todo list is at the bottom of requirements for merger.

This PR represents a shift in my mindset of how data should be modeled in an Ardite data system. Originally I wanted a "pure" JSON style of data modeling. However, when trying to implement the querying for this system, I realized how that would never ideally work.

Here are some advantages of a types/collection system over the flexible system we were building earlier:

One downside of this type approach is it may now be slightly harder to implement the GraphQL server.

Feel free to start reviewing so when I'm ready to merge the changes aren't too big (as of the writing of this message, this will likely be the most code which needs to be reviewed at once in the life cycle of this PR).

Things to complete before merging:

Edit: This PR has pivoted to just custom types and the read method so the above checklist does not apply.

calebmer commented 8 years ago

I'm actually going to pivot with this PR. I wasn't planning on adding MongoDB support in this PR, but I think it will help me better understand the implications of the new types I introduced with the read method on driver.

So, once you are ready to merge lets do it and I'll create a new PR for a MongoDB implementation and the todo list will be a repo level list and not a PR level list.

svmnotn commented 8 years ago

OK, I don't see anything glaringly wrong. Merge it.

calebmer commented 8 years ago

Your cool with type_?

svmnotn commented 8 years ago

I can't come up with a better name, and I don't think you are all that willing to change it. If anything I'll do a refactor when I come up with it.

calebmer commented 8 years ago

Ok, just make sure to run it by me first 👍