ipld / specs

Content-addressed, authenticated, immutable data structures
Other
592 stars 108 forks source link

Potential schema-schema changes. #291

Closed warpfork closed 3 years ago

warpfork commented 3 years ago

This is going to be a "draft" PR for discussion -- if anything comes of it, it'll probably be done by breaking out smaller diffs after discussion.

These are some potential changes to the schema-schema that I've developed while exercising things over in https://github.com/ipld/go-ipld-prime/pull/76 .

There are also some renames which I felt improved clarity:

I haven't altered the schema-schema DSL file to match yet. That diff would be quite a bit smaller (since it wouldn't have the indent change effect from the switch of union representation).

warpfork commented 3 years ago

@rvagg I'll be particularly interested in your thoughts on this. I think we've discussed some of these changes before, but now they're drafted to be seen in full. What are our impressions?

rvagg commented 3 years ago

Maybe we need to chat about this real-time. I'm probably not going to block these changes, but the removal of "kind" is a little annoying, it means you have to fish around inside the map for a list of keys, one of which will presumably tell you what it is, no more switch (thing.kind) {}. I think I've mentioned my hesitation about Unit when you raised it before too. It seems like a very Go-thinking concept that has dubious utility in here outside of Go codegen (which is not entirely unreasonable, but let's attempt to be a little balanced). I'd like to see a clearer justification for requiring an additional thing to handle this case. It would be good if you could cook up definitions for TypeUnit and whatever else you need to represent it.

vmx commented 3 years ago

(it's not really related to the topic, but might be useful though, GitHub supports diffs without whitespace only changes: https://github.com/ipld/specs/pull/291/files?diff=split&w=1)

warpfork commented 3 years ago

Unit actually comes from more of a haskelly view than a Golang view, fwiw. It's not common thinking in the Go world at all. :) Wikipedia's Unit type article is also good.

But point taken, I do need to write about it a bit more fully somewhere.

EDIT: that is now in https://github.com/ipld/specs/issues/292

warpfork commented 3 years ago

... I'm going to turn this into a merge-ignore, because I want the content for future reference, but I'm not working on this right now and will not push it to completion (and not necessarily in this exact form) anytime in the immediately coming days.

We'll see a "merge" here in the github UI shortly, but it will be a product of git checkout master && git merge -s ours this-branch, which means the content diff will not be taking actual effect on master.

warpfork commented 3 years ago

...ah, or not, because I pushed the merge before pushing the rebase I also did. Well, whatever. Closing, then. Same difference from the git POV, which I care about a great deal more than the github view.

warpfork commented 2 years ago

(Much of this, though not all, and also many other things, happened, in https://github.com/ipld/ipld/pull/136 .)