[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE

GitBox Wed, 26 Oct 2022 11:46:36 -0700


sonatype-lift[bot] commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1006066767



##########
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/RequestContinuesRecorderAuthenticationHandler.java:
##########
@@ -25,9 +25,10 @@
 import org.apache.hadoop.security.authentication.server.AuthenticationHandler;
 import org.apache.hadoop.security.authentication.server.AuthenticationToken;
 
-/**
- * {@link AuthenticationHandler} that delegates to another {@link 
AuthenticationHandler} and records
- * the response of managementOperation (which indicates whether the request 
should continue or not).
+/*
+ * {@link AuthenticationHandler} that delegates to another {@link 
AuthenticationHandler}
+ * and records the response of managementOperation (which indicates whether 
the request
+ * should continue or not).
  */
 public class RequestContinuesRecorderAuthenticationHandler implements 
AuthenticationHandler {

Review Comment:
   *[AlmostJavadoc](https://errorprone.info/bugpattern/AlmostJavadoc):*  This 
comment contains Javadoc or HTML tags, but isn't started with a double asterisk 
(/**); is it meant to be Javadoc?
   
   ---
   
   
   ```suggestion
   /**
   ```
   
   
   
   ---
   
   <details><summary><b>ℹ️ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with 
***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this 
PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified 
`file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see 
its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) 
to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=5)
 ]



##########
solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsQueryTemplate.java:
##########
@@ -23,20 +23,21 @@
 import java.util.regex.Pattern;
 
 public class MetricsQueryTemplate {
-  /**
-   * A regex with named groups is used to match template references to 
template + vars using the
-   * basic pattern:
-   *
-   * <p>$jq:<TEMPLATE>( <UNIQUE>, <KEYSELECTOR>, <METRIC>, <TYPE> )
-   *
-   * <p>For instance,
-   *
-   * <p>$jq:core(requests_total, endswith(".requestTimes"), count, COUNTER)
-   *
-   * <p>TEMPLATE = core UNIQUE = requests_total (unique suffix for this 
metric, results in a metric
-   * named "solr_metrics_core_requests_total") KEYSELECTOR = 
endswith(".requestTimes") (filter to
-   * select the specific key for this metric) METRIC = count TYPE = COUNTER
-   */
+  /*
+  A regex with named groups is used to match template references to template + 
vars using the basic pattern:
+
+      $jq:<TEMPLATE>( <UNIQUE>, <KEYSELECTOR>, <METRIC>, <TYPE> )
+
+  For instance,
+
+      $jq:core(requests_total, endswith(".requestTimes"), count, COUNTER)
+
+  TEMPLATE = core
+  UNIQUE = requests_total (unique suffix for this metric, results in a metric 
named "solr_metrics_core_requests_total")
+  KEYSELECTOR = endswith(".requestTimes") (filter to select the specific key 
for this metric)
+  METRIC = count
+  TYPE = COUNTER
+  */
   private static final Pattern matchJqTemplate =

Review Comment:
   *[AlmostJavadoc](https://errorprone.info/bugpattern/AlmostJavadoc):*  This 
comment contains Javadoc or HTML tags, but isn't started with a double asterisk 
(/**); is it meant to be Javadoc?
   
   ---
   
   
   ```suggestion
     /**
   ```
   
   
   
   ---
   
   <details><summary><b>ℹ️ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with 
***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this 
PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified 
`file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see 
its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) 
to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=5)
 ]



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -59,6 +59,14 @@ public Object doWork(Object... values) throws IOException {
       Number lagValue = 1;
 
       if (1 == values.length) {
+        if (!(timeseriesValues instanceof List<?>)) {

Review Comment:
   *[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):*  
`timeseriesValues` is an instance of List<Number> which is a subtype of 
List<?>, so this is equivalent to a null check.
   
   ---
   
   
   ```suggestion
           if (timeseriesValues == null) {
   ```
   
   
   
   ---
   
   <details><summary><b>ℹ️ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with 
***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this 
PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified 
`file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see 
its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) 
to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=5)
 ]



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -106,7 +114,7 @@ public Object doWork(Object... values) throws IOException {
 
       if (2 == values.length) {
         lagValue = (Number) values[1];
-        if (lagValue == null) {
+        if (!(lagValue instanceof Number)) {

Review Comment:
   *[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):*  
`lagValue` is an instance of Number which is a subtype of Number, so this is 
equivalent to a null check.
   
   ---
   
   
   ```suggestion
           if (lagValue == null) {
   ```
   
   
   
   ---
   
   <details><summary><b>ℹ️ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with 
***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this 
PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified 
`file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see 
its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) 
to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=5)
 ]



##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -70,7 +78,7 @@ public Object doWork(Object... values) throws IOException {
       }
       if (2 == values.length) {
         lagValue = (Number) values[1];
-        if (lagValue == null) {
+        if (!(lagValue instanceof Number)) {

Review Comment:
   *[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):*  
`lagValue` is an instance of Number which is a subtype of Number, so this is 
equivalent to a null check.
   
   ---
   
   
   ```suggestion
           if (lagValue == null) {
   ```
   
   
   
   ---
   
   <details><summary><b>ℹ️ Learn about @sonatype-lift commands</b></summary>
   
   You can reply with the following commands. For example, reply with 
***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this 
PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified 
`file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see 
its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) 
to add LiftBot to another repo.</sub></details>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=5)
 ]



-- 
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

[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE

Reply via email to