ctubbsii commented on code in PR #2843:
URL: https://github.com/apache/thrift/pull/2843#discussion_r1291627190
##########
lib/java/src/main/java/org/apache/thrift/protocol/TType.java:
##########
@@ -34,6 +34,11 @@ public final class TType {
public static final byte MAP = 13;
public static final byte SET = 14;
public static final byte LIST = 15;
- public static final byte ENUM = 16;
- public static final byte UUID = 17;
+ public static final byte UUID = 16;
+
+ /**
+ * This is not part of the TBinaryProtocol spec but Java specific
+ * implementation detail
+ */
+ public static final byte ENUM = -1;
Review Comment:
I'm not sure what ENUM was used for... but it seems like changing this will
break wire compatibility under some circumstances when using Thrift with Java.
I'm not even sure why there needs to be a separate UUID type at all... UUIDs
are just 128-bit numbers, and often hex-encoded as Strings. They can be
replaced by two 8-byte numbers, a 36 character String (or 32 without the
standard, but unnecessary, dashes), or a 16 byte array.
Since you can already pass a UUID in several different ways, creating a
separate dedicated type seems like it may have caused more problems than it
helps.
--
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]
