[GitHub] [solr] cpoerschke commented on a change in pull request #162: SOLR-14920: contrib/jaegertracer-configurator: apply & enforce 'spotless' code formatting

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org