> On Aug. 10, 2015, 5:01 p.m., Grant Henke wrote:
> > core/src/main/scala/kafka/server/TopicCommandHelper.scala, line 35
> > <https://reviews.apache.org/r/34766/diff/2/?file=995963#file995963line35>
> >
> >     A lot of this code/functionality exists in kafka.admin.TopicCommand. Is 
> > this duplicating a lot of code/functionality? Could they both be refactored 
> > to use the same core code?

IMO no, they cannot be refactored. The main problem is that code in 
kafka.admin.TopicCommand is very tightly coupled with CLI logic (like printing 
results, validation inputs), also error handling logic is very different.


- Andrii


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34766/#review94759
-----------------------------------------------------------


On June 30, 2015, 1:59 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34766/
> -----------------------------------------------------------
> 
> (Updated June 30, 2015, 1:59 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2229
>     https://issues.apache.org/jira/browse/KAFKA-2229
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KIP-4 Phase 1 Rebase
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
>   clients/src/main/java/org/apache/kafka/common/ConfigEntry.java PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/PartitionReplicaAssignment.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/TopicConfigInfo.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 
> b39e9bb53dd51f1ce931691e23aabc9ebaebf1e7 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 5b898c8f8ad5d0469f469b600c4b2eb13d1fc662 
>   clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 
> 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 
>   clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java 
> 5d3d52859587ce0981d702f04042b0f6e1bc3704 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicRequest.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicResponse.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicRequest.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicResponse.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicRequest.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicResponse.java
>  PRE-CREATION 
>   
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java
>  8b2aca85fa738180e5420985fddc39a4bf9681ea 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> f06edf41c732a7b794e496d0048b0ce6f897e72b 
>   core/src/main/scala/kafka/api/RequestKeys.scala 
> 155cb650e9cffe2c950326cfc25b1480cda819db 
>   core/src/main/scala/kafka/common/InvalidPartitionsException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/common/InvalidReplicaAssignmentException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/common/InvalidReplicationFactorException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/common/InvalidTopicConfigurationException.scala 
> PRE-CREATION 
>   
> core/src/main/scala/kafka/common/ReassignPartitionsInProgressException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> ad6f05807c61c971e5e60d24bc0c87e989115961 
>   core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 
>   core/src/test/scala/unit/kafka/server/TopicCommandHelperTest.scala 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34766/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>

Reply via email to