search

digidem / comapeo-schema

Data schemas for mapeo data types
MIT License
2 stars 0 forks source link

feat: add `remoteDetectionAlert` doc type #234

Closed tomasciccola closed 1 month ago

tomasciccola commented 2 months ago

This adds the remoteDetectionAlert doc type. The geometry message is a stub for now, until @gmaclennan implements its decoding conversion to GeoJSON.

The only doubt I have is around sourceId. Is there a clear format for this string? Should we add concrete validation for it?

Also, I reused the tags type for metadata (since they are structurally the same?, basically a Record<string, any>). I know that semantically is confusing, but this allows to reuse all the conversion functions regarding tags

should close #228

gmaclennan commented 2 months ago

This depends on #248 (because @comapeo/geometry doesn't export fromPartial methods, and it's easier to remove them from here than try to write then for Geometry), so that should be merged first.

The actual changes to add geometry are in https://github.com/digidem/mapeo-schema/pull/234/commits/7733276587d1d57109e502e635f914a7c9a896cc

It is a bit more complicated than I would like because of the need to deal with JSONSchema refs with 3 different libraries that we use (ajv, @json-schema-tools/dereferencer and @apidevtools/json-schema-ref-parser).

Would be nice to replace @json-schema-tools/dereferencer with @apidevtools/json-schema-ref-parser at some point, because the latter is already a dep of json-schema-to-typescript and we need to deal with it for that.

EvanHahn commented 2 months ago

I plan to review this next week.

tomasciccola commented 1 month ago

I think we should not merge this until we have released the MVP, in case we need bug fixes before MVP (having this merged would complicate bug fixes)

wouldn't it make sense to have a 'develop' branch where we merge this so we don't have a bunch of dangling open PRs (but maybe there won't be many of this)?