cpoerschke commented on PR #1883: URL: https://github.com/apache/solr/pull/1883#issuecomment-1712010145
Hi @tomgl - thank you very much for your inputs! > As for my understanding you plan to remove the `LegacyFieldValueFeature` **and** `useDocValuesForStored` in 9.X or 10, correct? Yes, the plan is to remove `LegacyFieldValueFeature` in a future version and the plan **was** to remove `useDocValuesForStored` too, **however** ... > Looking at the code, I think that people would have to copy quite a bit of code if they want to still use the legacy behaviour if useDocValuesForStored is gone. ... You raise a good point w.r.t. `useDocValuesForStored` removal being restrictive and actually retaining it adds near-zero code complexity and would facilitate anyone's custom retention of the legacy behaviour. And perhaps even a protected flag is clearer than a variant `FieldValueFeature` constructor? So https://github.com/apache/solr/pull/1883/commits/31eff548ae0535d48b92a9f9f343a0ed17ef78de is to retain the flag then. > I [refactored the tests a bit](https://github.com/apache/solr/commit/ae6d160176ea3c41854641ee1336d752ebe4db6c), to make it easier to see what `test_storedDvIsTrendy_FieldValueFeatureScorer_className_differs` wants to test and added some helper functions for `testThatCorrectFieldValueFeatureIsUsedForDocValueTypes` to (subjectively) improve readability. Thanks for that! Have added it to the branch and built on it with the https://github.com/apache/solr/pull/1883/commits/1a5b7b716b96a09b521ffc3d6a56823239ac4284 commit - WDYT? -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
