search

carbon-io / carbond

MIT License
2 stars 5 forks source link

Simplifying config schema nomenclature #292

Closed tfogo closed 6 years ago

tfogo commented 6 years ago

Currently most of the config objects have schemas named after their operation handler. So saveConfig has saveSchema. updateObjectConfig has updateObjectSchema etc.

It would be cleaner for the relevant config objects to just have a schema property. We don't need to differentiate that it's an updateObjectSchema if it's on updateObject.

I think this would simplify documentation and improve understandability. Thoughts?

willshulman commented 6 years ago

Would have to look but I think it is possible to have two schemas (maybe UpdateConfig has querySchema and updateSchema)? I guess the latter could just be 'schema'. Worth a discussion.

tfogo commented 6 years ago

querySchema doesn't exist. The schema for the query parameter is defined on queryParameter. It's the same with all other parameters. The OperationParameter class has a schema property.

willshulman commented 6 years ago

Ah that's right. Query is not a Collection concept, just a MongoDBCollection concept. Ok let's review together in our next meeting and make the call.

gregbanks commented 6 years ago

the reason for this naming is exactly as will suggests. we may want to define multiple schemas within the config (e.g., querySchema in MongoDBUpdateConfig).

i think updateObjectSchema should be renamed to updateSchema would be more consistent as it would map to the parameter that it is injected into.

gregbanks commented 6 years ago

https://github.com/carbon-io/carbond/pull/324