cpoerschke commented on a change in pull request #162:
URL: https://github.com/apache/solr/pull/162#discussion_r646906653



##########
File path: 
solr/contrib/jaegertracer-configurator/src/test/org/apache/solr/jaeger/TestJaegerConfigurator.java
##########
@@ -53,26 +51,23 @@ public void doBefore() {
 
   @Test
   public void testInjected() throws Exception {
-    MiniSolrCloudCluster cluster = new SolrCloudTestCase.Builder(2, 
createTempDir())
-        .addConfig("config", 
TEST_PATH().resolve("collection1").resolve("conf"))
-        .withSolrXml(getFile("solr/solr.xml").toPath())
-        .build();
+    MiniSolrCloudCluster cluster =
+        new SolrCloudTestCase.Builder(2, createTempDir())
+            .addConfig("config", 
TEST_PATH().resolve("collection1").resolve("conf"))
+            .withSolrXml(getFile("solr/solr.xml").toPath())
+            .build();
     try {
       TimeOut timeOut = new TimeOut(2, TimeUnit.MINUTES, TimeSource.NANO_TIME);
       timeOut.waitFor(
           "Waiting for GlobalTracer is registered",
           () -> GlobalTracer.get().toString().contains("JaegerTracer"));
 
-      //TODO add run Jaeger through Docker and verify spans available after 
run these commands
+      // TODO add run Jaeger through Docker and verify spans available after 
run these commands
       CollectionAdminRequest.createCollection("test", 2, 
1).process(cluster.getSolrClient());
-      new UpdateRequest()
-          .add("id", "1")
-          .add("id", "2")
-          .process(cluster.getSolrClient(), "test");
+      new UpdateRequest().add("id", "1").add("id", 
"2").process(cluster.getSolrClient(), "test");

Review comment:
       > ... How are the rules configured for overrides? ... I'd like to 
understand more about the plugin we're applying.
   
   From what I understand 
https://github.com/apache/solr/blob/a9a8d2023de00474277c6edbaff75d8bd12f33e8/gradle/validation/spotless.gradle#L37-L41
 is the area where the configuration happens and as per 
https://github.com/diffplug/spotless/tree/main/plugin-gradle there are many 
options available (though from a little bit of "browsing around" i couldn't 
find one that obviously covers the situation here).
   
   Oh and please note that `spotless.gradle` is also being changed as part of 
this pull request (to remove the exemption for 
`contrib/jaegertracer-configurator`) but the github UI wouldn't let me add a 
comment on the relevant lines, hence commenting here where the question cropped 
up.

##########
File path: 
solr/contrib/jaegertracer-configurator/src/test/org/apache/solr/jaeger/TestJaegerConfigurator.java
##########
@@ -37,8 +36,7 @@
 
 public class TestJaegerConfigurator extends SolrTestCaseJ4 {
 
-  @Rule
-  public TestRule solrTestRules = new SystemPropertiesRestoreRule();
+  @Rule public TestRule solrTestRules = new SystemPropertiesRestoreRule();

Review comment:
       > Is this option configurable?
   
   Experimentally if `solrTestRules` is changed to be a different name so that 
the line with the `@Rule` prepended is quite a bit longer then it leaves the 
`@Rule` on the previous line.
   
   I also note that 
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations says "A 
_single_ parameterless annotation _may_ instead appear together with the first 
line of the signature" i.e. "_may_" not "_must_" and that the Lucene code with 
the same configuration has a mix of prior and same line `@Rule` style.




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

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

Reply via email to