ctubbsii commented on a change in pull request #2558:
URL: https://github.com/apache/accumulo/pull/2558#discussion_r823044360
##########
File path:
core/src/main/java/org/apache/accumulo/core/client/summary/CounterSummary.java
##########
@@ -54,7 +54,8 @@ public CounterSummary(Summary summary, boolean checkType) {
if (checkType) {
String className = summary.getSummarizerConfiguration().getClassName();
try {
-
getClass().getClassLoader().loadClass(className).asSubclass(CountingSummarizer.class);
+ var unusedRetVal =
+
getClass().getClassLoader().loadClass(className).asSubclass(CountingSummarizer.class);
Review comment:
This might make errorprone happy, but it adds compiler warnings about
unused variables, at least in Eclipse (maybe also with javac?). One option is
to actually use it. Could add a trace log message that uses it after it is
assigned.
##########
File path: test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java
##########
@@ -468,7 +468,7 @@ public void testDeniedAudits() throws
AccumuloSecurityException, AccumuloExcepti
auditAccumuloClient.tableOperations().offline(OLD_TEST_TABLE_NAME);
} catch (AccumuloSecurityException ex) {}
try (Scanner scanner =
auditAccumuloClient.createScanner(OLD_TEST_TABLE_NAME, auths)) {
- scanner.iterator().next().getKey();
+ var unusedRetVal = scanner.iterator().next().getKey();
} catch (RuntimeException ex) {}
Review comment:
It looks like this is a pattern where we are expecting the error. This
should be refactored to use `assertThrows` instead of trying to ignore the
return value.
##########
File path:
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java
##########
@@ -91,7 +91,7 @@ public boolean equals(Object o) {
}
public final void precomputeHashCode() {
- hashCode();
+ var unusedRetVal = hashCode();
Review comment:
Instead of doing this, could just do this computation in the constructor
and delete this method. The only place where this is used is immediately after
construction in one case, and the only other time this object is constructed is
in ServerClient where it is not going to hurt to precompute.
##########
File path: test/src/main/java/org/apache/accumulo/test/SampleIT.java
##########
@@ -490,7 +490,7 @@ private void assertSampleNotPresent(SamplerConfiguration
sc, ScannerBase... scan
scanner.clearSamplerConfiguration();
for (Entry<Key,Value> entry : scanner) {
- entry.getKey();
+ var unusedRetVal = entry.getKey();
Review comment:
The call to `.getKey` was only ever added to avoid a previous warning
about the loop variable being unused. It would probably be sufficient to have
an assert that the loop variable is non-null.
##########
File path: test/src/main/java/org/apache/accumulo/test/CloseScannerIT.java
##########
@@ -58,7 +58,7 @@ public void testManyScans() throws Exception {
for (int j = 0; j < i % 7 + 1; j++) {
// only read a little data and quit, this should leave a session
open on the tserver
- Iterators.get(scanner.iterator(), 10);
+ var unusedRetVal = Iterators.get(scanner.iterator(), 10);
Review comment:
An assertNotNull() or similar around this would probably suffice.
##########
File path:
test/src/main/java/org/apache/accumulo/test/DetectDeadTabletServersIT.java
##########
@@ -56,7 +56,8 @@ protected void configure(MiniAccumuloConfigImpl cfg,
Configuration hadoopCoreSit
public void test() throws Exception {
try (AccumuloClient c =
Accumulo.newClient().from(getClientProperties()).build()) {
log.info("verifying that everything is up");
- Iterators.size(c.createScanner(MetadataTable.NAME,
Authorizations.EMPTY).iterator());
+ var unusedRetVal =
+ Iterators.size(c.createScanner(MetadataTable.NAME,
Authorizations.EMPTY).iterator());
Review comment:
This could verify the size is `> 0`.
--
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]
