You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/11/16 15:57:37 UTC
[GitHub] [pulsar] Shoothzj opened a new pull request #12837: Remove werid assign value in DefaultMessageFormatter
Shoothzj opened a new pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837
### Modifications
Remove werid assign value in DefaultMessageFormatter
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [x] `no-need-doc`
simple optimize
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#discussion_r750853678
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
Review comment:
It seems that it's a default size for error situation.
Maybe it's better not change it if you are absolute sure?
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
- size = 20;
- };
- String result = "";
+ StringBuilder result = new StringBuilder();
for(int i = 0; i < s; i++) {
Review comment:
`org.apache.commons.lang3.RandomStringUtils#randomAlphabetic(int)` can be used to generate random string.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #12837: Remove unused assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#discussion_r755188918
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
- size = 20;
- };
- String result = "";
+ StringBuilder result = new StringBuilder();
for(int i = 0; i < s; i++) {
Review comment:
Optimized
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#issuecomment-970844057
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#issuecomment-974145422
@Shoothzj Please check the comments
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #12837: Remove unused assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #12837: Remove unused assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#issuecomment-984345303
@Jason918 @RobertIndie Please help review this PR.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] RobertIndie commented on a change in pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
RobertIndie commented on a change in pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#discussion_r751201923
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
Review comment:
It seems that the `size` is not used later. It looks like the intent of this method is to generate the random string of default size 20 when 0 is passed in. I think the value here should be assigned to the `s` instead of the `size`.
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
- size = 20;
- };
- String result = "";
+ StringBuilder result = new StringBuilder();
for(int i = 0; i < s; i++) {
Review comment:
I agree.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] RobertIndie commented on a change in pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
RobertIndie commented on a change in pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#discussion_r751201923
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
Review comment:
It seems that the `size` is not used later. It looks like the intent of this method is to generate the random string of default size 20 when 0 is passed in. I think the value here should be assigned to the `s` instead of the `size`. But I prefer to use the existing random string implementation.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #12837: Remove werid assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#discussion_r755185574
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/DefaultMessageFormatter.java
##########
@@ -81,21 +81,18 @@ private float _getFloatValue(float size) {
private String getStringValue(float size) {
int s = (int) size;
- if (size == 0) {
Review comment:
That's the test-performance-producer for `%5s` stands for 5 length random string. The command is already widely use in performance tests. For people who want to use length for 20, they can use `%20s`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #12837: Remove unused assign value in DefaultMessageFormatter
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12837:
URL: https://github.com/apache/pulsar/pull/12837#issuecomment-976647955
@codelipenghui @Jason918 Please take a view again
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org