cpoerschke commented on a change in pull request #228:
URL: https://github.com/apache/solr/pull/228#discussion_r673272242
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -53,6 +53,21 @@ public LTRRescorer(LTRScoringQuery scoringQuery) {
this.scoringQuery = scoringQuery;
}
+ final protected static Comparator<ScoreDoc> docComparator = (a, b) -> a.doc
- b.doc;
Review comment:
```suggestion
final private static Comparator<ScoreDoc> docComparator = (a, b) -> a.doc
- b.doc;
```
i.e. minimum necessary visibility.
And I've seen `Comparator.comparingInt` used elsewhere in the code base
recently and would be curious if using that here too might be an alternative,
what do you think?
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -131,31 +146,12 @@ public TopDocs rescore(IndexSearcher searcher, TopDocs
firstPassTopDocs,
}
protected static void sortByScore(ScoreDoc[] reranked) {
Review comment:
So now the `sortByScore` contains much less code than before. I wonder
if its future might look like this:
* step 1: mark it deprecated (in this pull request) so that no new code
starts calling it
* step 2: change existing code (in this pull request) to sort directly with
the new comparator (which already has protected visibility equivalent to the
visibility of this method)
* step 3: remove the deprecated method (after this pull request is merged)
on `main` branch only for Solr 9.0 onwards but retain it in `branch_8x` branch
for Solr 8.10 onwards, to maintain build backwards compatibility for anyone who
may have built a custom plugin with a custom rescorer.
```suggestion
@Deprecated
protected static void sortByScore(ScoreDoc[] reranked) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
