jrhenderson1988 commented on code in PR #2939:
URL: https://github.com/apache/iggy/pull/2939#discussion_r2937517214
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/system/ClientInfoDetails.java:
##########
@@ -30,7 +29,7 @@ public record ClientInfoDetails(
String transport,
Long consumerGroupsCount,
List<ConsumerGroupInfo> consumerGroups) {
- public ClientInfoDetails(ClientInfo clientInfo,
ArrayList<ConsumerGroupInfo> consumerGroups) {
+ public ClientInfoDetails(ClientInfo clientInfo, List<ConsumerGroupInfo>
consumerGroups) {
Review Comment:
I assume it was a mistake to only allow an `ArrayList` in the constructor
here. The property is a `List<ConsumerGroupInfo>`. Changing this to a list
makes it more open than it was before, so it shouldn't be breaking in any way.
Again, happy to reverse this if preferred
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/exception/IggyException.java:
##########
@@ -46,13 +46,4 @@ protected IggyException(String message) {
protected IggyException(String message, Throwable cause) {
super(message, cause);
}
-
- /**
- * Constructs a new IggyException with the specified cause.
- *
- * @param cause the cause of the exception
- */
- protected IggyException(Throwable cause) {
Review Comment:
I found that there were no usages of this constructor by subclasses - and
since this class is abstract, it's not really reachable. It couldn't be
properly tested within making a new class that used it (or an anonymous class).
Technically a consumer of the SDK might be able to make their own subclass of
`IggyException` but I am not sure if that would make much sense in the context
use.
Therefore, I removed it. I'd be happy to put it back if preferred
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/topic/CompressionAlgorithm.java:
##########
@@ -31,7 +31,7 @@ public enum CompressionAlgorithm {
this.code = code;
}
- public static CompressionAlgorithm fromCode(byte code) {
+ public static CompressionAlgorithm fromCode(int code) {
Review Comment:
Also noticed this when writing a test. The property stored in the `enum` is
an `Integer` and all other enums use `int` in their `fromCode` methods. I
assumed this was a mistake too, so I updated it here. Again, happy to reverse
it.
--
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]
