[GitHub] [pulsar] tuteng commented on a change in pull request #4117: [pulsar-clients]Store key part of a KeyValue schema into pulsar message keys

[GitBox <gi...@apache.org>]

tuteng commented on a change in pull request #4117: [pulsar-clients]Store key part of a KeyValue schema into pulsar message keys
URL: https://github.com/apache/pulsar/pull/4117#discussion_r279175266
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/KeyValueSchema.java
 ##########
 @@ -98,17 +134,29 @@ private KeyValueSchema(Schema<K> keySchema,
         Gson valueSchemaGson = new Gson();
         properties.put("value.schema.properties", valueSchemaGson.toJson(valueSchema.getSchemaInfo().getProperties()));
 
+        if (keyValueEncodingType != null && keyValueEncodingType.equals(KeyValueEncodingType.SEPARATED)) {
 
 Review comment:
   > If you don't allow passing in a null value, you don't need if-else here. your logic is much simpler as below:
   > 
   > ```
   > this.keyValueEncodingType = this.keyValueEncodingType = KeyValueEncodingType.SEPARATED;;
   > properties.put("kv.encoding.type", String.valueOf(keyValueEncodingType));
   > ```
   
   Is that what you mean? @sijie 
   ```
   this.keyValueEncodingType = (keyValueEncodingType == null || keyValueEncodingType.equals(KeyValueEncodingType.SEPARATED)) ? keyValueEncodingType : KeyValueEncodingType.INLINE;
   properties.put("kv.encoding.type", String.valueOf(this.keyValueEncodingType));
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services