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/07/29 09:54:11 UTC
[GitHub] [pulsar] skyguard1 opened a new pull request #11497: Fix to use == instead of equals to compare string (#11462)
skyguard1 opened a new pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497
Fix to use == instead of equals to compare strings in PulsarColumnMetadata and ZooKeeperUtil
Fixes #11462
--
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] skyguard1 commented on pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-903829112
/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] skyguard1 commented on pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-903839868
/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] BewareMyPower commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r693987114
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
It looks like this comment is still not addressed. Could you address this comment and rebase to master?
--
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] 315157973 commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-889946894
/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] skyguard1 commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r693992516
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
This problem has been solved, see https://github.com/apache/pulsar/pull/11497#discussion_r685712017, the failure of ci build seems to have nothing to do with this pr, can you rerun the ci build?
--
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] BewareMyPower merged pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
BewareMyPower merged pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497
--
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] Anonymitaet commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-891514250
Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] nodece commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-890643296
/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] skyguard1 commented on a change in pull request #11497: Fix uses equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r684567405
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
I've done 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a change in pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r679177129
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!path.equals("")) {
Review comment:
Use `!"".equals(path)` instead of `!path.equals("")` for avoid null pointer.
##########
File path: pulsar-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarColumnMetadata.java
##########
@@ -187,7 +187,7 @@ public boolean equals(Object o) {
DecoderExtraInfo that = (DecoderExtraInfo) o;
- if (mapping != that.mapping) {
+ if (!mapping.equals(that.mapping)) {
Review comment:
Please check the null pointer.
--
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] skyguard1 commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-891560732
In my opinion, this fix will not change the functions, so there is no need to update the doc, thanks
--
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] eolivelli commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r685711727
##########
File path: pulsar-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarColumnMetadata.java
##########
@@ -187,7 +188,7 @@ public boolean equals(Object o) {
DecoderExtraInfo that = (DecoderExtraInfo) o;
- if (mapping != that.mapping) {
+ if (!StringUtils.equals(mapping, that.mapping)) {
Review comment:
no need to use a third party library for this.
we can use `Object.equals(mapping, that.mapping)`
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +116,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (StringUtils.isNotEmpty(path)) {
Review comment:
`if (!path.isEmpty()) ` is enough here, no need to use a third party library
--
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] skyguard1 commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r685721685
##########
File path: pulsar-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarColumnMetadata.java
##########
@@ -187,7 +188,7 @@ public boolean equals(Object o) {
DecoderExtraInfo that = (DecoderExtraInfo) o;
- if (mapping != that.mapping) {
+ if (!StringUtils.equals(mapping, that.mapping)) {
Review comment:
I also changed other comparisons to use `Objects.equals`, PTAL
--
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] skyguard1 commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-889577315
@nodece PTAL
--
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] michaeljmarshall commented on a change in pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r684524851
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
Why not use `StringUtils.isNotEmpty`? This utility is used very often in the project.
--
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] BewareMyPower commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r693995650
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
Sorry I didn't notice that comment. I'll rerun the CI first. BTW, you can also comment `/pulsarbot run-failure-checks` to achieve the same goal.
--
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] skyguard1 commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r686440022
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +116,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (StringUtils.isNotEmpty(path)) {
Review comment:
Don't care, it's not a big thing
--
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] skyguard1 commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r693998331
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +115,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (!"".equals(path)) {
Review comment:
Don't mind, thanks for your suggestion
--
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] nodece removed a comment on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
nodece removed a comment on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-890643296
/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] skyguard1 commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r685718996
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +116,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (StringUtils.isNotEmpty(path)) {
Review comment:
In fact, I changed to `StringUtils.isNotEmpty` because of @michaeljmarshall review.
https://github.com/apache/pulsar/pull/11497#discussion_r684524851
In my opinion, we should not use unnecessary third-party libraries. I think `! "".equals(path)` is also ok because `NullPointerException` can be avoided, thanks
--
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] skyguard1 commented on pull request #11497: Fix to use == instead of equals to compare string (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-894022727
@zymap Could you please re-run ci build? thanks
--
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] skyguard1 removed a comment on pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 removed a comment on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-903829112
/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] skyguard1 removed a comment on pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 removed a comment on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-903820276
/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] skyguard1 commented on pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
skyguard1 commented on pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#issuecomment-903820276
/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] michaeljmarshall commented on a change in pull request #11497: Use equals instead of == to compare strings (#11462)
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11497:
URL: https://github.com/apache/pulsar/pull/11497#discussion_r686313389
##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
##########
@@ -116,7 +116,7 @@ public void startServer(String path) throws Exception {
// create a zookeeper client
LOG.debug("Instantiate ZK Client");
zkc = ZooKeeperClient.newBuilder().connectString(getZooKeeperConnectString()).build();
- if (path != "") {
+ if (StringUtils.isNotEmpty(path)) {
Review comment:
Sorry for the back and forth here @skyguard1. I only recommended using `StringUtils.isNotEmpty` because it is used elsewhere in the project. `! "".equals(path)` certainly works, too.
--
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