cognitect-labs / vase

Data driven microservices
Eclipse Public License 1.0
374 stars 40 forks source link

Extend functionality of schema-tx reader literal #40

Closed nottmey closed 7 years ago

nottmey commented 7 years ago

Suggestion for 2 extentions of the #vase/schema-tx reader literal.

(1) Allow the use of :component and (2) allow definition of enum values as idents. e.g.:

[[:user/comment :many :ref :component "..."]
 [:user/role :one :ref "..."]
 [:user.role/admin "..."]
 [:user.role/moderator "..."]]

Any suggestions or concerns?

ohpauleez commented 7 years ago

Thanks for reaching out with these suggestions and jumping in to do the work!

In the future, consider opening an issue first before the pull-request so the community and Vase maintainers can all comment on use-cases/approaches/implementation ideas.

I've been considering adding isComponent support for some time, so this helps add more food for thought. I don't think the literal will ever support enums because those are already easily created. I'll let the general community offer up their opinions though.

Have you submitted a CLA yet?

nottmey commented 7 years ago

Hi! Just wanted to do some fine tuning with this one. Since it has very limited impact and design need. But sure, I will do it next time. I have signed the CLA a few hours ago. I separated the commits so the choice can be differentiated easily.

For me the :component option is a no-brainer, it's fairly common (?) and exclusive to the other optional toggles. It may be called something else though, e.g. :isComponent.

The enum change is optional of course. It just helps me to bundle attributes and enum values together, in the same syntax, without fragmenting the schema to much.

Tell me what you need or don't need and I finish this up.

ohpauleez commented 7 years ago

Master now contains changes to support :component as a qualifier in the shorthand literal. I'm going to close this PR, but feel free to open up an issue to support the enum shorthand.

Thanks!