cmccabe commented on code in PR #12050:
URL: https://github.com/apache/kafka/pull/12050#discussion_r874257133
##########
clients/src/main/java/org/apache/kafka/clients/NodeApiVersions.java:
##########
@@ -91,19 +95,11 @@ public static NodeApiVersions create(short apiKey, short
minVersion, short maxVe
.setMaxVersion(maxVersion)));
}
- public NodeApiVersions(ApiVersionCollection nodeApiVersions) {
- for (ApiVersion nodeApiVersion : nodeApiVersions) {
- if (ApiKeys.hasId(nodeApiVersion.apiKey())) {
- ApiKeys nodeApiKey = ApiKeys.forId(nodeApiVersion.apiKey());
- supportedVersions.put(nodeApiKey, nodeApiVersion);
- } else {
- // Newer brokers may support ApiKeys we don't know about
- unknownApis.add(nodeApiVersion);
- }
- }
+ public NodeApiVersions(Collection<ApiVersion> nodeApiVersions) {
Review Comment:
Is it necessary to have a special constructor for the case where there are
no supported features? This won't be a common situation in the future, right?
This feels like the kind of thing that could easily lead to mistakes, if
someone accidentally uses it outside of test code.
Looking at my IDE I see 10 uses of this in test code. Can we just change
them to use the two-argument form?
--
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]
