benwtrent commented on code in PR #13311:
URL: https://github.com/apache/lucene/pull/13311#discussion_r1569381543
##########
lucene/core/src/test/org/apache/lucene/search/BaseKnnVectorQueryTestCase.java:
##########
@@ -781,7 +781,7 @@ public void testTimeLimitingKnnCollectorManager() throws
IOException {
TimeLimitingKnnCollectorManager noTimeoutManager =
new TimeLimitingKnnCollectorManager(delegate, null);
KnnCollector noTimeoutCollector =
- noTimeoutManager.newCollector(Integer.MAX_VALUE,
searcher.leafContexts.getFirst());
+ noTimeoutManager.newCollector(Integer.MAX_VALUE,
searcher.leafContexts.get(0));
Review Comment:
> I was worried about recurring conflicts if we just change it in branch_9x,
is that not the case?
I wouldn't expect the conflicts to be to horrible. The biggest ones I have
ran into is the new `switch` statement usages.
> Skipping backport for something like this didn't seem right either.
No, backporting the test fix is good. Allows them to be unmuted & ensures we
are actually testing these paths.
> I'd like to learn what we consider as good practice for Lucene codebase.
There is no prevalent pattern. Its a "gut" thing, and I would rather there
not be any mandate.
Its fine that you did this, just wanted to indicate that in this particular
case, seemed like it wasn't necessary.
--
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...@lucene.apache.org
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
