Add Message Data Model #195
Conversation
Co-authored-by: Dan Minor <dminor@mozilla.com>
|
Updated the proposal based on requests by @mihai, separating the abstract data type definitions of the interfaces from their canonical JSON equivalents as well as applying other requested changes, such as dropping references to metadata objects. The proposed structure is now hopefully capable of reaching the appropriate level of abstraction, as we define nine types/interfaces instead of only five. |
|
@mihnita Do you have any remaining concerns about this PR? |
There was a problem hiding this comment.
@mihnita Do you have any remaining concerns about this PR?
Sorry, yes.
I got hit (again) by the non-intuitive github UI.
I though I submitted the review, but I needed to look at the commit with Firefox and not logged-in to see that they are not actually pushed.
:-(
|
This would be a great first PR where we can:
|
|
Updated the PR following discussions on today's brainstorming call, based on review comments from @mihnita and @stasm:
We further agreed on the call that @mihnita and @stasm will re-review this PR within the next 24h or so, allowing it to finally be merged. |
There was a problem hiding this comment.
LGTM, great work @eemeli and others. The last time I looked at this was probably a month ago and I scribbled down some notes about dropping meta, JSON, type and a few others. I'm sorry that I didn't have time to participate more actively in the discussion in this PR. OTOH I see that other reviewers had similar comments; reviewing this today I don't have much to add. I think this PR represents a good starting point for future spec work.
| Each of the SelectMessage `cases` is defined by a key of one or more string identifiers, | ||
| and selection between them is made according to the corresponding Selector values. | ||
| From this it follows that a valid SelectMessage must have at least as many `select` entries | ||
| as its highest count of string entries within the keys of its `cases`. |
There was a problem hiding this comment.
(Non-blocking) I'd suggest to drop the From this it follows... sentence. We'll need to figure this out anyways when specifying the case selection logic, and I don't know if such a requirement will be needed.
As a first step in actually filling out the spec, here's my proposal for the Message data model.
This should be relatively uncontroversial, as we've previously agreed that messages may have top-level selectors. This data model is also as simple as possible, and does not e.g. allow for
numbervalues in selector keys. I'll later submit a PR for a selector algorithm that works with the proposed data model.The definitions of PatternElement and Meta are left outside the scope of this PR.
As a stylistic choice, I've opted here to use normal text when referring to defined objects (which have a Capital first letter), but format their properties and methods as
code. The types & interfaces use TypeScript for clarity.