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



core/src/main/scala/kafka/security/auth/PermissionType.scala (line 47)
<https://reviews.apache.org/r/34492/#comment146474>

    The ``return`` here is redundant. In fact the L#46 - L#48 could be 
rewritten as either:
    
    ```
    def values() : List[PermissionType] = {
       List(Allow, Deny)
    }
    ```
    
    or just
    
    ```
    def values = List(Allow, Deny)
    ```
    
    please, check with the committers what they prefer. In any case the return 
is a unnecessary. ;)


- Edward Ribeiro


On July 20, 2015, 11:42 p.m., Parth Brahmbhatt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34492/
> -----------------------------------------------------------
> 
> (Updated July 20, 2015, 11:42 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2210
>     https://issues.apache.org/jira/browse/KAFKA-2210
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Addressing review comments from Jun.
> 
> 
> Adding CREATE check for offset topic only if the topic does not exist already.
> 
> 
> Addressing some more comments.
> 
> 
> Removing acl.json file
> 
> 
> Moving PermissionType to trait instead of enum. Following the convention for 
> defining constants.
> 
> 
> Adding authorizer.config.path back.
> 
> 
> Addressing more comments from Jun.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/api/OffsetRequest.scala 
> f418868046f7c99aefdccd9956541a0cb72b1500 
>   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> c75c68589681b2c9d6eba2b440ce5e58cddf6370 
>   core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/KafkaPrincipal.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 18f5b5b895af1469876b2223841fd90a2dd255e0 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> dbe170f87331f43e2dc30165080d2cb7dfe5fdbf 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 18917bc4464b9403b16d85d20c3fd4c24893d1d3 
>   core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/KafkaPrincipalTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/OperationTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/ResourceTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 
> 04a02e08a54139ee1a298c5354731bae009efef3 
> 
> Diff: https://reviews.apache.org/r/34492/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Parth Brahmbhatt
> 
>

Reply via email to