dnhatn commented on pull request #134:
URL: https://github.com/apache/lucene/pull/134#issuecomment-839934931


   @rmuir @jpountz Thanks for reviewing.
   
   > Can we add tests to BaseStoredFieldsFormatTestCase that call setIndexSort, 
exercise merging, and validate correctness (e.g. check that stored fields 
values are still correct)? I am worried that existing test coverage for this 
change is not great.
   
   I added a random test in 
https://github.com/apache/lucene/pull/134/commits/be7c71c320be86f5e485a6d0864b18954f5ef25f.
   
   > Can we also fix merge logic of 
Lucene90TermVectorsFormat/BaseTermVectorsFormatTestCase? Generally, we try to 
keep this logic consistent across stored fields and term vectors as much as 
possible, and this improvement should apply there as well?
   
   +1. Are you okay if I do will it in a follow-up right after this PR?
   
   Would you please take another look?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to