search

blockades / scuttle-dark-crystal

API for validating, building, publishing and reading Dark Crystal records
http://darkcrystal.pw
MIT License
42 stars 2 forks source link

Development (WIP / tracking PR) #30

Closed mixmix closed 5 years ago

mixmix commented 5 years ago

this is to help remember what the diff is btwn dev and master (because I'm having trouble keeping it all in ma head).

I imagine ultimately this will get reviewed and merged.

mixmix commented 5 years ago

STATUS : I'm reviewing and doing minor modifications. Please do not touch this branch till I'm done

ameba23 commented 5 years ago

Great work mix. you spotted some serious bugs and i really like what you've done with the secrets wrapper, will for sure make it easier to update in the future and for others to read / maintain.

i see you've merged the prune dead wood stuff. are you happy with that? i kind of wasn't really finished looking for dead wood, don't know if i would have found more but am happy if its off the list and we can go on to more interesting things...

ameba23 commented 5 years ago

one other really little thing thats been on my mind:

with regards to our validateShard method in the secrets wrapper: i'm pretty sure we can write a regex which catches everything that secrets.extractShareComponents would catch, which means we could include it directly in the JSON schema, which would be much neater. although it would need to be a different regex for v1/v2 not sure if that would be complicated to do.

mixmix commented 5 years ago

@ameba23 I don't have my head around the validateShard part so much yet. I'll trust that you know best about that. I guess I'd hazard that if it's anything but the simplest thing, writing good regex can be a dangerous endeavour.

mixmix commented 5 years ago

OH JEEZ ... I started seperating tests to demarcate which versions of things they're testing. I'm getting pretty spaced out on this, but what I think I've discovered is:

Proposal:

cc @ameba23

mixmix commented 5 years ago

THIS is in a deliberately failing state. This block of work is done, I won't push anything more to this branch. Anyone doing more work will be working via PRs

I have split out needed work to get this into a healthy state in these cards :

ameba23 commented 5 years ago
  • EITHER change the forward version to match the shard version you're handling
  • OR (probably this one) add a shardVersion prop to dark-crystal/forward schema

I guess we gotta take the second option... the first option is kind of nice and neat, but if we later change the forward message schema somehow we'll get into a right muddle. so we need version for the forward message, and nested version of the shard it contains. aargh!

mixmix commented 5 years ago

merged and published as v2.0.0