You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "clolov (via GitHub)" <gi...@apache.org> on 2023/02/07 11:31:07 UTC
[GitHub] [kafka] clolov opened a new pull request, #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
clolov opened a new pull request, #13212:
URL: https://github.com/apache/kafka/pull/13212
I ran IntelliJ IDEA's Code Inspection and corrected occurrences of the following:
* https://www.jetbrains.com/help/idea/list-of-java-inspections.html#javadoc (declaration has problems in Javadoc references)
* https://www.jetbrains.com/help/idea/list-of-java-inspections.html#performance (bulk operation can be used instead of iteration)
* https://www.jetbrains.com/help/idea/list-of-java-inspections.html#inheritance-issues (unnecessary call to toString())
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] clolov commented on pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on PR #13212:
URL: https://github.com/apache/kafka/pull/13212#issuecomment-1440432643
This has been rebased on the latest trunk, test have been ran locally and I would be very grateful for a review when you get the time @mimaison
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] clolov commented on a diff in pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on code in PR #13212:
URL: https://github.com/apache/kafka/pull/13212#discussion_r1099881594
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -779,8 +779,7 @@ public static ByteBuffer ensureCapacity(ByteBuffer existingBuffer, int newLength
@SafeVarargs
public static <T> Set<T> mkSet(T... elems) {
Set<T> result = new HashSet<>((int) (elems.length / 0.75) + 1);
- for (T elem : elems)
- result.add(elem);
+ result.addAll(Arrays.asList(elems));
Review Comment:
Yeah, the tests failed yesterday, so I will be reverting that particular commit
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -779,8 +779,7 @@ public static ByteBuffer ensureCapacity(ByteBuffer existingBuffer, int newLength
@SafeVarargs
public static <T> Set<T> mkSet(T... elems) {
Set<T> result = new HashSet<>((int) (elems.length / 0.75) + 1);
- for (T elem : elems)
- result.add(elem);
+ result.addAll(Arrays.asList(elems));
Review Comment:
Yeah, the tests failed yesterday as well, so I will be reverting that particular commit
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mimaison merged pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison merged PR #13212:
URL: https://github.com/apache/kafka/pull/13212
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on a diff in pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on code in PR #13212:
URL: https://github.com/apache/kafka/pull/13212#discussion_r1098802862
##########
clients/src/main/java/org/apache/kafka/clients/admin/AlterClientQuotasOptions.java:
##########
@@ -19,6 +19,8 @@
import org.apache.kafka.common.annotation.InterfaceStability;
+import java.util.Collection;
Review Comment:
where are we using this in this file?
(same for other imports)
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -779,8 +779,7 @@ public static ByteBuffer ensureCapacity(ByteBuffer existingBuffer, int newLength
@SafeVarargs
public static <T> Set<T> mkSet(T... elems) {
Set<T> result = new HashSet<>((int) (elems.length / 0.75) + 1);
- for (T elem : elems)
- result.add(elem);
+ result.addAll(Arrays.asList(elems));
Review Comment:
we are creating an intermediate data structure (list) here in the new code. It might not be worth it.
(same for other places where we use Arrays.asList)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] clolov commented on a diff in pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on code in PR #13212:
URL: https://github.com/apache/kafka/pull/13212#discussion_r1099881202
##########
clients/src/main/java/org/apache/kafka/clients/admin/AlterClientQuotasOptions.java:
##########
@@ -19,6 +19,8 @@
import org.apache.kafka.common.annotation.InterfaceStability;
+import java.util.Collection;
Review Comment:
Javadocs was complaining that it doesn't know about this. The same for the other imports 😊
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] clolov commented on pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on PR #13212:
URL: https://github.com/apache/kafka/pull/13212#issuecomment-1420626007
@divijvaidya for visibility
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] clolov commented on pull request #13212: MINOR: Remove accidental unnecessary code; fix comment references; use bulk operations
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on PR #13212:
URL: https://github.com/apache/kafka/pull/13212#issuecomment-1420762600
```
[2023-02-07T11:33:01.745Z] > Task :clients:compileJava
[2023-02-07T11:33:01.745Z] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-13212/clients/src/main/java/org/apache/kafka/common/utils/Utils.java:782: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter elems
[2023-02-07T11:33:01.745Z] result.addAll(Arrays.asList(elems));
[2023-02-07T11:33:01.745Z] ^
[2023-02-07T11:33:01.745Z] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-13212/clients/src/main/java/org/apache/kafka/common/utils/Utils.java:795: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter elems
[2023-02-07T11:33:01.745Z] result.addAll(Arrays.asList(elems));
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org