You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/06/03 21:46:13 UTC
[GitHub] [kafka] afalko opened a new pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
afalko opened a new pull request #8794:
URL: https://github.com/apache/kafka/pull/8794
NioEchoServer the enum has its constructor declared as private, which is redundant. There is also an unused exception throws and unused method.
Reviewers: Jakob Homan <jg...@gmail.com>
This is a newbie ticket to get used to the contribution flow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] afalko commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
afalko commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-638478890
@jghoman
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-716115322
@afalko Thanks for updating code. Are those failed tests related to this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] afalko commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
afalko commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-716195452
> @afalko Could you rebase PR to trigger QA?
Thanks @chia7712
> @afalko Thanks for updating code. Are those failed tests related to this patch?
@chia7712 These are not, last time it ran the jdk8 and jdk11 tests passed, but the jdk15 tests failed, this time it is the other way around.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] afalko commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
afalko commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-716195533
I'll merge master once more
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] afalko commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
afalko commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-716058925
> @afalko Could you rebase PR to trigger QA?
Thanks! Looks like it partially passed. I'll merge master against and see if it does better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-712557078
@afalko Could you rebase PR to trigger QA?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jghoman commented on pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
jghoman commented on pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#issuecomment-639882793
ok to test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #8794:
URL: https://github.com/apache/kafka/pull/8794#discussion_r434944964
##########
File path: clients/src/test/java/org/apache/kafka/common/network/NioEchoServer.java
##########
@@ -60,14 +60,8 @@
public enum MetricType {
TOTAL, RATE, AVG, MAX;
- private final String metricNameSuffix;
-
- private MetricType() {
- metricNameSuffix = "-" + name().toLowerCase(Locale.ROOT);
- }
-
public String metricNameSuffix() {
- return metricNameSuffix;
+ return "-" + name().toLowerCase(Locale.ROOT);
Review comment:
This replaces a single allocation during construction with allocations on every method call.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 merged pull request #8794: KAFKA-10092: Remove unused code branches in NioEchoServer
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #8794:
URL: https://github.com/apache/kafka/pull/8794
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org