> On June 9, 2017, 7:55 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/59869/diff/7/?file=1746528#file1746528line72>
> >
> >     This is not used by anything

Yeah I left it as is so it could come in handy in future.


> On June 9, 2017, 7:55 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
> > Lines 100 (patched)
> > <https://reviews.apache.org/r/59869/diff/7/?file=1746528#file1746528line100>
> >
> >     What is the result if we have 2,3,5,7? According to your tests, it is 
> > "2-3, 5-7" while I think it should be "2, 3, 5, 7"

FOr 2, 3, 5, 7 --> The function would return: [2-3, 5, 7]

I see that you are expecting two consecutive numbers to show up seperately. I 
will do this.
After this change, it would now return [2, 3, 5, 7]


> On June 9, 2017, 7:55 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 3763 (original), 3767 (patched)
> > <https://reviews.apache.org/r/59869/diff/7/?file=1746529#file1746529line3768>
> >
> >     Else after return is redundand and just makes things less readable.
> >     
> >     Is this == or != ? Seems like you inverted the condition.

The review already has been updated with !=


- Vamsee


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


On June 9, 2017, 1:35 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59869/
> -----------------------------------------------------------
> 
> (Updated June 9, 2017, 1:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov and Lei Xu.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> * Changes on top of Lina's review (https://reviews.apache.org/r/59820/)
> * Adds a couple of helper methods that would go under MSentryUtil
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryChange.java
>  6011ef407aaf82d211c81f6d6a55975fb21261b9 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
>  7558267546fc8c4dedc4f739df6092851becfc31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  17856a4425be5c8971c51f7cfd9b2ef06001a2ab 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59869/diff/7/
> 
> 
> Testing
> -------
> 
> In progress.
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>

Reply via email to