Copilot commented on code in PR #4743:
URL: https://github.com/apache/solr/pull/4743#discussion_r3797626365
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/search/LTRQParserPlugin.java:
##########
@@ -55,7 +55,6 @@
*/
public class LTRQParserPlugin extends QParserPlugin
implements ResourceLoaderAware, ManagedResourceObserver {
- public static final String NAME = "ltr";
private static final String ORIGINAL_RANKING = "_OriginalRanking_";
Review Comment:
Removing this public constant is a source-incompatible API change for
integrations that reference `LTRQParserPlugin.NAME`, and it departs from the
established query-parser convention (for example, `ReRankQParserPlugin.java:48`
and `KnnQParserPlugin.java:29`). Keep the constant rather than deleting it as
an unused implementation detail.
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/RegexRulesPasswordProvider.java:
##########
@@ -103,20 +103,6 @@ public static LinkedHashMap<Pattern, String>
parseRulesFile(InputStream is) {
return rules;
}
- /**
- * Initialize rules through file input stream. This is a convenience for
first calling
- * setPasswordMap(parseRulesFile(is)).
- *
- * @param is the input stream with rules file, one line per rule on format
regex=password
- */
- public void parse(InputStream is) {
- setPasswordMap(parseRulesFile(is));
- }
-
- public LinkedHashMap<Pattern, String> getPasswordMap() {
- return passwordMap;
- }
-
public void setPasswordMap(LinkedHashMap<Pattern, String> linkedHashMap) {
this.passwordMap = linkedHashMap;
}
Review Comment:
Deleting the public `parse(InputStream)` and `getPasswordMap()` methods
breaks external callers of this public password-provider class.
Repository-local usage is not sufficient to establish that a public API is
unused; retain these methods (or deprecate them through the compatibility
process) in a tidy-only change.
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/RegexRulesPasswordProvider.java:
##########
@@ -138,9 +124,4 @@ public String getExplicitPassword() {
public void setExplicitPassword(String explicitPassword) {
this.explicitPassword = explicitPassword;
}
-
- /** Resets explicit password, so that map will be used for lookups */
- public void resetExplicitPassword() {
- this.explicitPassword = null;
- }
}
Review Comment:
Deleting the public reset method is another source-incompatible change for
users that reuse a provider after setting an explicit password. Retain it or
deprecate it before removal.
--
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]