Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

Add weekly sync meeting notes 2020-01-20 #52

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Conversation

vmx
Copy link
Member

@vmx vmx commented Jan 20, 2020

No description provided.

- Dag* formats (like DagCBOR): sort in a certain way in write time, on write time they just read in how it was written => ordered
- Validation should be applied on our strict formats (DAG*) on read - it's an error if the keys appear out of order
- ( riba ) When writing spec make abs. sure **unicode normal form** is specified
- dissent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"dissent" - I meant to question this during the meeting - who added this, @warpfork? can you expand on this, it's a bit of a landmine sitting in here like this.
Or was this just a note that there was dissent but it wasn't significant enough to extrapolate on?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't even notice this: definitely worth clarifying before merge...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry that was me.

I think we SHOULD make remarks about UTF-8 + NFC normalization being the desired format of strings.

I also think we should use the words "SHOULD" and not "MUST".

Copy link
Contributor

@warpfork warpfork Jan 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Everything MUST be defined to work in a defined way on byte comparison, regardless -- or our system would contain undefined behavior, so this is truly nonoptional
  • It is the "fortunate" (well-designed) case that sorting UTF-8 strings bytewise is correct anyway
  • If the above two weren't enough, it's worth noting that we already have good company in Canonical CBOR's spec, which demands bytewise sorting
  • I had a conversation at a conference in which I said to someone "I'm dealing with unicode normalization issues" and they said "can you just... not" and I was frankly unable to uphold my position against such a simple argument.

I know -- I was in support for aggressive normalization and rule enforcement when this was asked around this time last year. I've updated my position. I'm no longer convinced that there are good things coming of trying to make strings be more than bytes.

Copy link
Contributor

@warpfork warpfork Jan 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have read the https://www.unicode.org/reports/tr15/#Problem_Sequence_Table, I have seen the , and I no longer believe its our place to police strings in this project nor do I want to be the one to do it.

Reliably round-tripping the user's bytes is a more viable strategy in every way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I previously did a roundup issue on this at ipld/specs#67 and that is now also a record frozen at https://github.com/ipld/specs/blob/master/design/history/exploration-reports/2018.07-merkle-segments.md . We should probably update those (if only to say the original author no longer stands by half that) -- or if we really wanna explore this again, start drafting a new exploration report and take that through a comment cycle... and then, yeah, ideally get this into spec somewhere.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is going to get lost and needs to be recorded somewhere more permanent. High quality rant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make sure that this conversation isn't lost. I'll open an issue/PR on the specs repo once ipld/specs#236 is merged. I leave this PR open until everything is merged so that we can link it from the notes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The promised PR: ipld/specs#350

Copy link
Contributor

@ribasushi ribasushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending resolution of #52 (comment)

vmx added a commit to ipld/specs that referenced this pull request Jan 15, 2021
This change was triggered by ipld/team-mgmt#52 (review) and I think this is the right place to put that information.
vmx added a commit to ipld/specs that referenced this pull request Jan 29, 2021
This change was triggered by ipld/team-mgmt#52 (review) and I think this is the right place to put that information.
@vmx vmx merged commit 1cc44f2 into master Jan 29, 2021
@vmx vmx deleted the weekly-sync-2020-01-20 branch January 29, 2021 14:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants