You are viewing a plain text version of this content. The canonical link for it is here.
Posted to devnull@infra.apache.org by "omkreddy (GitHub)" <gi...@apache.org> on 2018/08/23 19:27:44 UTC
[GitHub] [kafka] omkreddy opened pull request #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporter) props to documentation
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] lindong28 commented on pull request #5563:
KAFKA-5066: Add KafkaMetricsConfig (Yammer metrics reporters) props to
documentation
Posted by "lindong28 (GitHub)" <gi...@apache.org>.
Would it be more readable and consistent with existing code style by moving these variables (property name, doc and default value) to KafkaConfig.java?
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] lindong28 closed pull request #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporters) props to documentation
Posted by "lindong28 (GitHub)" <gi...@apache.org>.
[ pull request closed by lindong28 ]
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] lindong28 commented on pull request #5563:
KAFKA-5066: Add KafkaMetricsConfig (Yammer metrics reporters) props to
documentation
Posted by "lindong28 (GitHub)" <gi...@apache.org>.
Thanks for the update. Just one more minor comment. Would it be slightly better to also define `KafkaMetricsReporterClasses = ""` in `kafka.serverDefaults`, similar to the existing `Defaults.MetricReporterClasses = ""`? It seems that in the existing KafkaConfig.java, we always use use a default variable with value `""` rather than using `""` directly. Otherwise LGTM.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] lindong28 commented on issue #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporters) props to documentation
Posted by "lindong28 (GitHub)" <gi...@apache.org>.
Thanks for the patch @omkreddy :)
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] omkreddy commented on issue #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporters) props to documentation
Posted by "omkreddy (GitHub)" <gi...@apache.org>.
@junrao @ijuma minor PR for doc purpose.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] omkreddy commented on pull request #5563:
KAFKA-5066: Add KafkaMetricsConfig (Yammer metrics reporters) props to
documentation
Posted by "omkreddy (GitHub)" <gi...@apache.org>.
@lindong28 Thanks for the review. Updated the PR.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] omkreddy commented on issue #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporters) props to documentation
Posted by "omkreddy (GitHub)" <gi...@apache.org>.
@hachikuji Please take a look at this minor change for doc purpose.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] omkreddy commented on pull request #5563:
KAFKA-5066: Add KafkaMetricsConfig (Yammer metrics reporters) props to
documentation
Posted by "omkreddy (GitHub)" <gi...@apache.org>.
@lindong28 Makes sense. Updated the PR to make it consistent with existing configs.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] omkreddy commented on issue #5563: KAFKA-5066: Add
KafkaMetricsConfig (Yammer metrics reporters) props to documentation
Posted by "omkreddy (GitHub)" <gi...@apache.org>.
@lindong28 Please take a look whenever you get a chance. This is a minor change for doc purpose.
[ Full content available at: https://github.com/apache/kafka/pull/5563 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org