Add annotations field to output MessageDefinitions from parseIdl
#48
Conversation
snosenzo
commented
Aug 9, 2023
snosenzo
commented
- keep annotations on MessageDefinitions for usage in serialization and deserialization
- remove extra array wrap for result of AST
- consolidate IDLNodeProcessor logic
…nd deserialization - remove extra array wrap for result of AST - consolidate IDLNodeProcessor logic
| ) { | ||
| const isEnum = node.declarator === "enum"; | ||
| const members = isEnum ? node.enumerators : node.definitions; | ||
| const definitionFields = members | ||
| .map((def) => |
There was a problem hiding this comment.
Not something for this PR and likely not a concern for the amount of data involved here, but one easy performance win is to avoid making closures inside of tight loops. Maybe harder to avoid here cause you are using a value outside of the closure but something to keep an eye out for.
|
|
||
| const idlProcessor = new IDLNodeProcessor(rawDefinitions); | ||
| idlProcessor.resolveEnumTypes(); |
There was a problem hiding this comment.
What's the benefit of having to call all these separate functions? What happens if you forget to call these? Do you end up with malformed or incomplete message definitions? If the answer is yes then I think its worth making it easier to do the right thing. Mayhe that's what this parseRos2Idl does.
Something else worth noting is whether you really want to make IDLNodeProcessor part of the public interface.
- have two different `parseToIdl` functions: one for annotated defs and other for not-annotated
snosenzo
deleted the
sam/fg-4326-add-annotation-tags-to-resolved-idl-definitions-for-use-in
branch
