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]

Reply via email to