Re: Review Request 31590: Patch for KAFKA-1990
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31590/ --- (Updated April 19, 2015, 5:41 p.m.) Review request for kafka and Gwen Shapira. Bugs: KAFKA-1990 https://issues.apache.org/jira/browse/KAFKA-1990 Repository: kafka Description --- KAFKA-1990 Diffs (updated) - core/src/main/scala/kafka/log/LogConfig.scala 558c703f26da22b1a938bbbf8a6c4409a8e107fb core/src/main/scala/kafka/log/LogManager.scala a7a9b85ba1b80c2b32afe6edbbc175f08238809c core/src/main/scala/kafka/server/KafkaConfig.scala 69b772c1941865fbe15b34bb2784c511f8ce519a core/src/test/scala/kafka/log/LogConfigTest.scala 9690f141be75202973085025444b52208ebd5ab2 core/src/test/scala/unit/kafka/admin/AdminTest.scala cfe38df577e3f179ebecad3f45429a15aa69e7b4 core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 40c265aabae8e09b1d0958802ef7e2fc97689d11 Diff: https://reviews.apache.org/r/31590/diff/ Testing --- Updated for ConfigDef. Open question. Is there any reason that LogConfigTest is located in kafka/log rather than unit/kafka/log ? Thanks, Jeff Holoman
Re: Review Request 31590: Patch for KAFKA-1990
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31590/#review80526
---
Thanks for the patch. A few comments below.
Also, yes, LogConfigTest should be in unit/kafka/log instead of kafka/log.
Could you fix it in this patch too?
core/src/main/scala/kafka/log/LogManager.scala
https://reviews.apache.org/r/31590/#comment130516
See the coding convention comment below.
core/src/main/scala/kafka/log/LogManager.scala
https://reviews.apache.org/r/31590/#comment130504
Our current coding convention is not to wrap single line statement with {}.
core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/31590/#comment130525
The original code is more idiomatic in scala.
low.
- Jun Rao
On March 10, 2015, 4:58 a.m., Jeff Holoman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31590/
---
(Updated March 10, 2015, 4:58 a.m.)
Review request for kafka and Gwen Shapira.
Bugs: KAFKA-1990
https://issues.apache.org/jira/browse/KAFKA-1990
Repository: kafka
Description
---
KAFKA-1990
Diffs
-
core/src/main/scala/kafka/log/LogConfig.scala
8b67aee3a37765178b30d79e9e7bb882bdc89c29
core/src/main/scala/kafka/log/LogManager.scala
47d250af62c1aa53d11204a332d0684fb4217c8d
core/src/main/scala/kafka/server/KafkaConfig.scala
48e33626695ad8a28b0018362ac225f11df94973
core/src/test/scala/kafka/log/LogConfigTest.scala
9690f141be75202973085025444b52208ebd5ab2
core/src/test/scala/unit/kafka/admin/AdminTest.scala
ee0b21e6a94ad79c11dd08f6e5adf98c333e2ec9
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
7f47e6f9a74314ed9e9f19d0c97931f3f2e49259
Diff: https://reviews.apache.org/r/31590/diff/
Testing
---
Updated for ConfigDef.
Open question. Is there any reason that LogConfigTest is located in kafka/log
rather than unit/kafka/log ?
Thanks,
Jeff Holoman
Re: Review Request 31590: Patch for KAFKA-1990
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31590/ --- (Updated March 10, 2015, 4:58 a.m.) Review request for kafka and Gwen Shapira. Bugs: KAFKA-1990 https://issues.apache.org/jira/browse/KAFKA-1990 Repository: kafka Description --- KAFKA-1990 Diffs - core/src/main/scala/kafka/log/LogConfig.scala 8b67aee3a37765178b30d79e9e7bb882bdc89c29 core/src/main/scala/kafka/log/LogManager.scala 47d250af62c1aa53d11204a332d0684fb4217c8d core/src/main/scala/kafka/server/KafkaConfig.scala 48e33626695ad8a28b0018362ac225f11df94973 core/src/test/scala/kafka/log/LogConfigTest.scala 9690f141be75202973085025444b52208ebd5ab2 core/src/test/scala/unit/kafka/admin/AdminTest.scala ee0b21e6a94ad79c11dd08f6e5adf98c333e2ec9 core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 7f47e6f9a74314ed9e9f19d0c97931f3f2e49259 Diff: https://reviews.apache.org/r/31590/diff/ Testing (updated) --- Updated for ConfigDef. Open question. Is there any reason that LogConfigTest is located in kafka/log rather than unit/kafka/log ? Thanks, Jeff Holoman
Re: Review Request 31590: Patch for KAFKA-1990
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31590/ --- (Updated March 10, 2015, 4:55 a.m.) Review request for kafka and Gwen Shapira. Summary (updated) - Patch for KAFKA-1990 Bugs: KAFKA-1990 https://issues.apache.org/jira/browse/KAFKA-1990 Repository: kafka Description (updated) --- KAFKA-1990 Diffs (updated) - core/src/main/scala/kafka/log/LogConfig.scala 8b67aee3a37765178b30d79e9e7bb882bdc89c29 core/src/main/scala/kafka/log/LogManager.scala 47d250af62c1aa53d11204a332d0684fb4217c8d core/src/main/scala/kafka/server/KafkaConfig.scala 48e33626695ad8a28b0018362ac225f11df94973 core/src/test/scala/kafka/log/LogConfigTest.scala 9690f141be75202973085025444b52208ebd5ab2 core/src/test/scala/unit/kafka/admin/AdminTest.scala ee0b21e6a94ad79c11dd08f6e5adf98c333e2ec9 core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 7f47e6f9a74314ed9e9f19d0c97931f3f2e49259 Diff: https://reviews.apache.org/r/31590/diff/ Testing --- Thanks, Jeff Holoman
