[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335598648
##
File path:
processing/src/main/java/org/apache/druid/query/topn/PooledTopNAlgorithm.java
##
@@ -50,17 +50,14 @@
/**
*/
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
public class PooledTopNAlgorithm
extends BaseTopNAlgorithm
{
- private static boolean SPECIALIZE_GENERIC_ONE_AGG_POOLED_TOPN =
- !Boolean.getBoolean("dontSpecializeGeneric1AggPooledTopN");
- private static boolean SPECIALIZE_GENERIC_TWO_AGG_POOLED_TOPN =
- !Boolean.getBoolean("dontSpecializeGeneric2AggPooledTopN");
- private static boolean
SPECIALIZE_HISTORICAL_ONE_SIMPLE_DOUBLE_AGG_POOLED_TOPN =
-
!Boolean.getBoolean("dontSpecializeHistorical1SimpleDoubleAggPooledTopN");
- private static boolean
SPECIALIZE_HISTORICAL_SINGLE_VALUE_DIM_SELECTOR_ONE_SIMPLE_DOUBLE_AGG_POOLED_TOPN
=
-
!Boolean.getBoolean("dontSpecializeHistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopN");
+ private static boolean SPECIALIZE_GENERIC_ONE_AGG_POOLED_TOPN =
!Boolean.getBoolean("dontSpecializeGeneric1AggPooledTopN");
Review comment:
Lines longer than 120 cols
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335591855 ## File path: extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java ## @@ -3199,8 +3199,7 @@ public ProtoTestEvent parsePartialFrom( return descriptor; } - private static com.google.protobuf.Descriptors.FileDescriptor - descriptor; + private static com.google.protobuf.Descriptors.FileDescriptor descriptor; Review comment: Don't edit generated class. You can change the scope of the inspection to `NonGeneratedFiles`. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335581582
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/query/ScanBenchmark.java
##
@@ -102,6 +102,10 @@
@Measurement(iterations = 25)
public class ScanBenchmark
{
+ @Param({"NONE", "DESCENDING", "ASCENDING"})
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
+ private static ScanQuery.Order ordering;
Review comment:
This field is not set in an initializer block. Please check the truthfulness
of the comments that you add.
Since this is `@Param`, it shouldn't be static at all. Could you please
prohibit static `@Params` by adding another structured search pattern:
```java
@Param($Values$) static $T$ $param$;
```
in this PR?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335596027
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java
##
@@ -38,8 +38,7 @@
public class HyperUniquesSerde extends ComplexMetricSerde
{
- private static Comparator comparator =
-
Comparator.nullsFirst(Comparator.comparing(HyperLogLogCollector::toByteBuffer));
+ private static final Comparator COMPARATOR =
Comparator.nullsFirst(Comparator.comparing(HyperLogLogCollector::toByteBuffer));
Review comment:
Line longer than 120 cols
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335583876
##
File path: core/src/main/java/org/apache/druid/utils/JvmUtils.java
##
@@ -50,11 +50,12 @@ public static boolean isIsJava9Compatible()
}
@Inject
- private static RuntimeInfo runtimeInfo = new RuntimeInfo();
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
Injection.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335585792
##
File path:
core/src/test/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandlerTest.java
##
@@ -43,6 +43,8 @@
private static final int TOTAL_BYTES = 1 << 10;
private static final ArrayList BYTE_LIST = new ArrayList<>();
private static final Random RANDOM = new Random(378134789L);
+
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
method
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335590256
##
File path:
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorTest.java
##
@@ -126,6 +126,7 @@
private static final StreamPartition SHARD2_PARTITION =
StreamPartition.of(STREAM, SHARD_ID2);
private static final StreamPartition SHARD3_PARTITION =
StreamPartition.of(STREAM, SHARD_ID3);
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
method
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r33559
##
File path:
processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
##
@@ -39,6 +39,7 @@
*/
public class MapLookupExtractionFnSerDeTest
{
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
method
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335585321
##
File path:
core/src/test/java/org/apache/druid/data/input/impl/prefetch/PrefetchableTextFilesFirehoseFactoryTest.java
##
@@ -60,6 +60,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
Review comment:
Typo: "**a** static method". Please fix all the similar places in this PR.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335587140
##
File path:
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java
##
@@ -84,13 +84,14 @@
import java.util.List;
import java.util.Map;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
public class ThetaSketchSqlAggregatorTest extends CalciteTestBase
{
private static final String DATA_SOURCE = "foo";
private static QueryRunnerFactoryConglomerate conglomerate;
Review comment:
This code repeats in a number of classes, could it be factored out to
`CalciteTestBase`?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335595612
##
File path:
indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
##
@@ -48,6 +48,7 @@
import java.util.regex.Pattern;
@RunWith(Parameterized.class)
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because not initialized in constructor.
Review comment:
Fields don't need to be static.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335591115
##
File path:
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/query/lookup/namespace/StaticMapExtractionNamespaceTest.java
##
@@ -32,6 +32,8 @@
{
private static final Map MAP = ImmutableMap.builder().put("foo", "bar").build();
private static final ObjectMapper MAPPER = new DefaultObjectMapper();
+
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
private static String MAP_STRING;
Review comment:
Seems like `setUpStatic()` actually could be inlined:
```java
private static final String MAP_STRING = MAPPER.writeValueAsString(MAP);
```
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335582594
##
File path: core/src/main/java/org/apache/druid/common/config/NullHandling.java
##
@@ -50,6 +50,7 @@
* It does not take effect in all unit tests since we don't use Guice
Injection.
*/
@Inject
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
Again, has nothing to do with an initializer block. In this case, it's
because this field is injected it cannot be final.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335601766
##
File path:
processing/src/test/java/org/apache/druid/query/scan/ScanResultValueTimestampComparatorTest.java
##
@@ -36,6 +36,7 @@
public class ScanResultValueTimestampComparatorTest
{
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
private static QuerySegmentSpec intervalSpec;
Review comment:
setup() can be inlined
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335599463 ## File path: processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java ## @@ -95,10 +95,17 @@ private CompressionFactory() * clearEncodingFlag function. */ + /* Review comment: This comment duplicates a comment above. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335595130
##
File path:
indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLifecycleTest.java
##
@@ -250,6 +250,7 @@ public int compare(DataSegment dataSegment, DataSegment
dataSegment2)
private SegmentHandoffNotifierFactory handoffNotifierFactory;
private Map> handOffCallbacks;
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
private static CountDownLatch publishCountDown;
Review comment:
This field doesn't need to be static.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335600321
##
File path:
processing/src/test/java/org/apache/druid/query/filter/IntervalDimFilterTest.java
##
@@ -37,6 +37,7 @@
public class IntervalDimFilterTest
{
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
method
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335583652
##
File path:
core/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java
##
@@ -40,6 +40,7 @@
public static final String EXCEPTION_MESSAGE_KEY = "exceptionMessage";
public static final String EXCEPTION_STACK_TRACE_KEY = "exceptionStackTrace";
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
Same problem here, the comment doesn't reflect the reality.
registerEmitter() is the reason.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335590033
##
File path:
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplierTest.java
##
@@ -51,6 +51,7 @@
import java.util.Set;
import java.util.stream.Collectors;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because not initialized in constructor
Review comment:
I don't understand what does this comment mean. Besides, I think all fields
starting from `recordsPerFetch` actually don't need to be static in this class.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335582995
##
File path:
core/src/main/java/org/apache/druid/java/util/common/io/NativeIO.java
##
@@ -35,9 +35,10 @@
/**
* Native I/O operations in order to minimize cache impact.
*/
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
Please move this suppression to `FIELD`.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335604366
##
File path:
processing/src/test/java/org/apache/druid/segment/SchemalessIndexTest.java
##
@@ -64,9 +64,10 @@
/**
*/
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
Review comment:
`mergedIndex` doesn't need to be static. It seems to be that `index` doesn't
need to be static, too. `getIncrementalIndex()` and `makeIncrementalIndex()`
should become non-static, too.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335600158
##
File path:
processing/src/test/java/org/apache/druid/query/filter/InDimFilterSerDesrTest.java
##
@@ -35,6 +35,8 @@
public class InDimFilterSerDesrTest
{
+
+ @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
method
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r328694201
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/FloatCompressionBenchmarkFileGenerator.java
##
@@ -42,6 +42,7 @@
import java.util.List;
import java.util.Map;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
Review comment:
If the suppression is done for a single field (`dirPath`), it should be on
the field still. My suggestion to move the suppressions up only applied when
there are more than one field in the class which needs to be suppressed.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r328694779
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/LongCompressionBenchmarkFileGenerator.java
##
@@ -42,6 +42,7 @@
import java.util.List;
import java.util.Map;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
Review comment:
If the suppression is done for a single field (`dirPath`), it should be on
the field still.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r328696122
##
File path: core/src/main/java/org/apache/druid/common/config/NullHandling.java
##
@@ -29,6 +29,7 @@
* introduced as part of https://github.com/apache/incubator-druid/issues/4349
and the old druid behavior
* where null values are replaced with default values e.g Null Strings are
replaced with empty values.
*/
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an static method
Review comment:
If the suppression is done for a single field (`INSTANCE`), it should be on
the field still.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r328694407 ## File path: benchmarks/src/main/java/org/apache/druid/benchmark/IncrementalIndexRowTypeBenchmark.java ## @@ -51,7 +51,7 @@ private IncrementalIndex incIndex; private IncrementalIndex incFloatIndex; private IncrementalIndex incStrIndex; - private static AggregatorFactory[] aggs; + private static final AggregatorFactory[] AGGS; Review comment: Please move static fields above instance fields. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r328695611
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/query/ScanBenchmark.java
##
@@ -100,6 +100,7 @@
@Fork(value = 1)
@Warmup(iterations = 10)
@Measurement(iterations = 25)
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because variable is not initilized.
Review comment:
If this is about `ordering` field, I think it could be an instance field.
The order of static and instance field is also disturbed.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r326897473
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/FloatCompressionBenchmarkFileGenerator.java
##
@@ -42,6 +42,7 @@
import java.util.List;
import java.util.Map;
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
Review comment:
The comment is not true: field `dirPath` is not set in an initializer block.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r326856015
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/FilterPartitionBenchmark.java
##
@@ -124,8 +124,8 @@
private BenchmarkSchemaInfo schemaInfo;
- private static String JS_FN = "function(str) { return 'super-' + str; }";
- private static ExtractionFn JS_EXTRACTION_FN = new
JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getEnabledInstance());
+ private static final String JS_FN = "function(str) { return 'super-' + str;
}";
Review comment:
Static fields should be ordered before instance fields.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field
leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r326855992
##
File path:
benchmarks/src/main/java/org/apache/druid/benchmark/FloatCompressionBenchmark.java
##
@@ -55,12 +55,15 @@
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public class FloatCompressionBenchmark
{
+ @SuppressWarnings("SSBasedInspection")
Review comment:
Please try to minimize these suppressions. In most *Benchmark and *Test
classes static fields could be converted into instance fields.
In classes where this cannot be done, it makes sense to add
`@SuppressWarnings("SSBasedInspection")` once on the class declaration instead
of separately on every field.
In any case, *each* `@SuppressWarnings("SSBasedInspection")` should have a
comment after it specifying the concrete type of Structural Search inspection
being suppressed. In this PR, many of them should all look like
```java
@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final
because set in an initializer block
```
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
