jhump commented on code in PR #3266:
URL: https://github.com/apache/avro/pull/3266#discussion_r1932320163


##########
lang/c++/include/avro/CustomAttributes.hh:
##########
@@ -19,27 +19,62 @@
 #ifndef avro_CustomAttributes_hh__
 #define avro_CustomAttributes_hh__
 
-#include "Config.hh"
 #include <iostream>
 #include <map>
 #include <optional>
 #include <string>
 
+#include "Config.hh"
+
 namespace avro {
 
 // CustomAttributes class stores avro custom attributes.
 // Each attribute is represented by a unique name and value.
 // User is supposed to create CustomAttributes object and then add it to 
Schema.
 class AVRO_DECL CustomAttributes {
+
 public:
-    // Retrieves the custom attribute json entity for that attributeName, 
returns an
-    // null if the attribute doesn't exist.
+    enum class ValueMode : uint8_t {
+        // When a CustomAttributes is created using this mode, all values are 
expected
+        // to be strings. The value should not be quoted, but any interior 
quotes and
+        // special characters (such as newlines) must be escaped.
+        STRING
+        [[deprecated("The JSON ValueMode is less error-prone and less 
limited.")]],

Review Comment:
   Do I need to add something to a 1.14 checklist or issue tracker to make sure 
it actually gets removed in 1.14?
   
   Also, a couple of questions about removal of deprecated items:
   1. Should we _also_ put extra language like this for the no-arg constructor?
   2. If we remove it and there is then only a single enum value, it feels 
strange to require people to supply that value to the constructor. Perhaps , 
instead of _removal_ of the no-arg constructor, we could change its behavior in 
a backwards incompatible way? Or maybe we need to remove it in 1.14 and then 
add it back in 1.15 with different/incompatible semantics?



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