jmckenzie-dev commented on code in PR #323:
URL: https://github.com/apache/cassandra-sidecar/pull/323#discussion_r2841895534


##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/CassandraClientTokenRingProviderTest.java:
##########
@@ -282,12 +282,12 @@ private static TokenRange createMockTokenRange(Token 
start, Token end)
         }
     }
 
-    protected DnsResolver mockDnsResolver()
+    public static DnsResolver mockDnsResolver()

Review Comment:
   Why the increase in scope from private to public? I don't see any new 
consumers in the PR...



##########
CHANGES.txt:
##########
@@ -1,5 +1,6 @@
 0.3.0
 -----
+ * Rangemanager should be singleton in CDCModule (CASSSIDECAR-411)

Review Comment:
   Super nit: `RangeManager` to match casing on class name. 😅 



##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/CassandraClientTokenRingProviderTest.java:
##########
@@ -308,21 +308,21 @@ private InstancesMetadata mockInstancesMetadata()
     {
         InstancesMetadata instancesMetadata = mock(InstancesMetadata.class);
 
-        InstanceMetadata instance1 = getMockInstanceMetaData(101000101, 
"local1", getMetadata());
-        InstanceMetadata instance2 = getMockInstanceMetaData(101000201, 
"local2", getMetadata());
-        InstanceMetadata instance3 = getMockInstanceMetaData(101000301, 
"local3", getMetadata());
+        InstanceMetadata instance1 = getMockInstanceMetaData(101000101, 
"localhost", getMetadata());
+        InstanceMetadata instance2 = getMockInstanceMetaData(101000201, 
"localhost2", getMetadata());
+        InstanceMetadata instance3 = getMockInstanceMetaData(101000301, 
"localhost3", getMetadata());
         when(instancesMetadata.instances()).thenReturn(List.of(instance1, 
instance2, instance3));
         return instancesMetadata;
     }
 
-    private Metadata getMetadata()
+    public static Metadata getMetadata()

Review Comment:
   Same as above - purpose of scope promotion? If intentional we should either 
annotated `@VisibleForTesting` or javadoc it w/@link so subsequent refactors 
pull this in / break / make noise in an obvious way.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to