rmuir commented on pull request #134: URL: https://github.com/apache/lucene/pull/134#issuecomment-839175711
Change looks great, a few questions: * 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. * 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? -- 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
