Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2155
@srdo I agree with you concerning this
[comment](https://github.com/apache/storm/pull/2155#issuecomment-311751331).
There is really not a much better way to do this. I agree with your comment
"move the FirstPollOffsetStrategy default into a static field in
KafkaSpoutConfig like the rest for consistency" - let's do it.
I think that what confused me is the way the whole class is organized. For
instance, the state of KafkaSpoutConfig is not at the top of the class as one
would expect. Perhaps we should follow the usual convention and move the
[constructor and
fields](https://github.com/srdo/storm/blob/142d1f8e2609d1a48b6be71084987fe8cdc46a3f/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L302-#L331)
to the top, after the constants, then put the static inner classes
([here](https://github.com/srdo/storm/blob/142d1f8e2609d1a48b6be71084987fe8cdc46a3f/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L59-#L79)
and
[here](https://github.com/srdo/storm/blob/142d1f8e2609d1a48b6be71084987fe8cdc46a3f/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L125-#L300)),
then the [factory
methods](https://github.com/srdo/storm/blob/142d1f8e2609d1a48b6be71084987fe8cdc46a3f/external/st
orm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L81-#L123),
and at last all the [accessor
methods](https://github.com/srdo/storm/blob/142d1f8e2609d1a48b6be71084987fe8cdc46a3f/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L333-#L398).
On a different note, I noticed that this PR now has the changes from the
[STORM-2541 PR](https://github.com/apache/storm/pull/2150) included in its
single commit. I think that this PR should have only one commit, with the
refactoring of KafkaSpoutConfig. The [STORM-2541
PR](https://github.com/apache/storm/pull/2150) should have two commits, this
PR's commit, and it's commit. That would make it easy to merge and the same
time not loose Git history.
---
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.
---
