dbt-labs / dbt-jsonschema

Apache License 2.0
109 stars 40 forks source link

allow optional + #55

Closed dave-connors-3 closed 1 year ago

dave-connors-3 commented 1 year ago

For any config, use patternProperties allow for an optional + at the start of the key name

closes #36

joellabes commented 1 year ago

@dave-connors-3 I think I tried doing this last year, and discovered that pattern-based columns prevent autocomplete from working in most systems, because the IDE doesn't know what it can sensibly offer up.

So I might be a buzzkill on this and suggest that we have to duplicate everything with and without pluses 😬 (or insist on people using the pluses? they are more correct, although since there's no equivalent to sqlfluff fix for a JSON Schema the migration would be painful for anyone trying to do that)

dave-connors-3 commented 1 year ago

i am in accordance with your feedback! autocomplete and highlighting on both versions feels like a better overall experience for people

dave-connors-3 commented 1 year ago

@joellabes ready for rereview!

joellabes commented 1 year ago

@dave-connors-3 here's some other thoughts, as I look at that big wall of copy-paste and grimace at the thought of keeping them in sync:

dave-connors-3 commented 1 year ago

@joellabes made the update we discussed -- for each config, there's now a reference in defs for the definition of that config, and then a plus and non-plus version that references that definition. Anything that already referenced a definition (like docs_config for example, was left alone, and the duplicated non-plus entry should point to the same definition.

joellabes commented 1 year ago

@dave-connors-3 this looks great! I have just added a CI test (you can see the results here, here and here if you'd like to see what the output is) to validate that every key exists with/without a plus, and that their contents are identical.

Strictly speaking, it is inefficient to validate that the objects are identical in both directions, but I have to check the keys have counterparts in both directions and decided that it didn't matter enough to optimise.

Therefore I've approved this but not merged it - you can make sure you're happy with my changes, and if you are then merge away!

dave-connors-3 commented 1 year ago

thank you @joellabes