Github user hmcl commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2155#discussion_r124572749
  
    --- Diff: 
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
 ---
    @@ -79,16 +78,46 @@
             UNCOMMITTED_EARLIEST,
             UNCOMMITTED_LATEST }
         
    +    /**
    +     * Convenience method to get a Builder for String key/value spouts. 
Sets the key/value Deserializers to the StringDeserializer class.
    --- End diff --
    
    NIT: I would suggest a more succinct javadoc, e.g. "Factory method that 
creates a Builder that with key/val String deserializers.". In my opinion the 
text mentioning how to use builders for other ser/des should go in the README.
    
    Same comment would apply to other Builder factory methods.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to