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


Fix it, then Ship it!




Looks good, and thank for the fix to the verify query method!

Some of the verifyQuery calls in shouldNotTokenizeWordsWithKeywordAnalyzerseem 
more like just examples and exploration of how the lucene query parser works, 
rather than asserts that are testing that geode uses the keyword analyzer. 
Maybe move them to another method or remove them?


geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
 (line 71)
<https://reviews.apache.org/r/47712/#comment199462>

    Remove the commented out code - and the TODO line.



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
 (line 140)
<https://reviews.apache.org/r/47712/#comment199463>

    In this test, it seems like you are not really using field1 for anything. 
Maybe just set that to the empty string and don't include it in the query? 
Otherwise it's just extra information someone has to wade through to understand 
the test.


- Dan Smith


On May 23, 2016, 6:22 a.m., xiaojian zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47712/
> -----------------------------------------------------------
> 
> (Updated May 23, 2016, 6:22 a.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: GEODE-1352
>     https://issues.apache.org/jira/browse/GEODE-1352
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> designed a lot cases to test query syntax related with analyzer
> 
> 
> Diffs
> -----
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
>  1e2b63d 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java
>  e589ef4 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java
>  a0319f4 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxLuceneSerializer.java
>  c5c55a9 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java
>  953f31f 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIntegrationTest.java
>  c302460 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  15f5747 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
>  dded69c 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java
>  70ec434 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
>  571049c 
> 
> Diff: https://reviews.apache.org/r/47712/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Reply via email to