Support Union definition processing #63
Conversation
|
@snosenzo is this ready for review or still a draft? I guess it's only a draft because it was stacked on the other PR? |
| return this.astNode.name; | ||
| } | ||
|
|
||
| get isComplex(): boolean { |
There was a problem hiding this comment.
Maybe an unpopular opinion but I don't like this "isComplex" attribute name. It's something we inherited from the old days of webviz ROS message support. The name doesn't seem to do a great job of describing what it means (since we don't really use the word "complex" elsewhere in our code so it's not really a well-known term). Maybe someday we can come up with a better name or a different way of representing this information.
There was a problem hiding this comment.
Yeah I definitely agree. It's really not evident how this is going to be used or what it means. Right now I'm just using it to signify that these types are not resolved like non-complex types and don't have direct, simple serializers. I'm using the aggregatedKind attribute (taken from the IDL/DDS spec) to augment this, but maybe there's a variable name that's better for MessageDefinition? i am trying to keep this mostly compliant with MessageDefinition for now. Food for thought for the future though
| get cases(): Case[] { | ||
| return this.astNode.cases.map((def) => { |
There was a problem hiding this comment.
What's the reasoning for implementing these as getters vs. computing them once and storing them as members?
There was a problem hiding this comment.
I wanted instances of these classes to also take the shape of their final resolved type so that when accessing them you don't have to worry about resolving. As for why I'm not computing and storing them once, I did want to memoize these with a decorator to reduce repetition, but the last PR ended up being too big so I'll do it in another PR.
| }, | ||
| }, | ||
| { | ||
| predicates: [1], |
There was a problem hiding this comment.
When is predicates.length != 1? Let's get some test cases covering that too.
snosenzo
force-pushed
the
sam/fg-4329-process-and-resolve-union-definition-for-serialization
branch
from
7329e5c
to
a1534dc
Compare
snosenzo
deleted the
sam/fg-4329-process-and-resolve-union-definition-for-serialization
branch
Support for resolving references in Union cases, switchType, or defaultCase.
FG-4329