martin-g commented on code in PR #3324:
URL: https://github.com/apache/avro/pull/3324#discussion_r1973077925


##########
lang/c++/include/avro/NodeImpl.hh:
##########
@@ -235,9 +235,9 @@ using NodeImplSymbolic = NodeImpl<HasName, NoLeaves, 
NoLeafNames, NoAttributes,
 using NodeImplRecord = NodeImpl<HasName, MultiLeaves, LeafNames, 
MultiAttributes, NoSize>;
 using NodeImplEnum = NodeImpl<HasName, NoLeaves, LeafNames, NoAttributes, 
NoSize>;

Review Comment:
   AFAIK the spec says that only the `primitive` types could not have 
attributes (custom or not).
   So, I think `enum` should support them too, just like `map` and `fixed`.
   
   About `union`s - since their members could be `primitive` I think they 
should not support custom attributes.
   
   I am not sure what `symbolic` is. Is that a reference to previously defined 
type ? IMO they should not support custom attributes too. These attributes 
could be defined in the target (record, enum, fixed).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to