maccamlc commented on a change in pull request #883:
URL: https://github.com/apache/avro/pull/883#discussion_r429716263



##########
File path: 
lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
##########
@@ -328,6 +328,11 @@ private void getClassNamesOfPrimitiveFields(Schema schema, 
Set<String> result, S
       break;
     case ENUM:
     case FIXED:

Review comment:
       Have done some testing, and unifying with `result.add(javaType(scheme, 
true))` appears perfectly logical. This method is only being used currently to 
determine the types that have conversions, and this will pick it up.
   
   It performs a bit of unnecessary work if no logical type for the fixed/enum, 
as goes through the mangle path, but at the same time, it probably makes the 
code more consistent and potentially easier to follow imo. 




----------------------------------------------------------------
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.

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


Reply via email to