You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by GitBox <gi...@apache.org> on 2022/12/10 09:47:05 UTC
[GitHub] [incubator-eventmesh] sohel-79 opened a new pull request, #2550: [Fixes] #2537
sohel-79 opened a new pull request, #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Cloese #<XXX>`.)
-->
Fixes #2537
### Motivation
Method stores the return result in a local variable, and then immediately returns the local variable. It would be simpler just to return the value that is assigned to the local variable
### Modifications
Made changes to method
located at:
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java line 151
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045192743
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
I understand that the original code is more readable and testable, and I will pay more attention to readability during the CR process. So I also want to know what is the main reason for modifying this code?
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045189645
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
I don't think this way of writing is more readable now.
For example, there are too many parentheses, also not testable.
@sohel-79
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] sohel-79 commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
sohel-79 commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045192261
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
Can you give any suggestions?
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045192743
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
I understand that the original code is more readable, and I will pay more attention to readability during the CR process. So I also want to know what is the main reason for modifying this code?
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] sohel-79 commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
sohel-79 commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045391746
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
Right, close this PR and also issue #2537
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] sohel-79 commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
sohel-79 commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045235564
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
This method stores the return result in a local variable, and then immediately returns the local variable. It would be simpler just to return the value that is assigned to the local variable.
Mentioned in issue #2537
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] xwm1992 closed pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
xwm1992 closed pull request #2550: [Fixes #2537 ] Method stores return result in local
URL: https://github.com/apache/incubator-eventmesh/pull/2550
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] sohel-79 commented on pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
sohel-79 commented on PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#issuecomment-1345205556
@Alonexc I did the changes but the build failed
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] xwm1992 commented on pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
xwm1992 commented on PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#issuecomment-1345791873
@sohel-79 these code changes seems more complicated and less readable.
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045209601
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
I will close this PR later if there is no good reason.
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] codecov[bot] commented on pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#issuecomment-1345349010
# [Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2550](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c83f589) into [master](https://codecov.io/gh/apache/incubator-eventmesh/commit/7f7e2f04f73f3ad9e10cec88e31c51d4638d7cd4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7f7e2f0) will **decrease** coverage by `0.02%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #2550 +/- ##
============================================
- Coverage 11.66% 11.63% -0.03%
+ Complexity 904 900 -4
============================================
Files 469 469
Lines 27901 27892 -9
Branches 3026 3017 -9
============================================
- Hits 3254 3245 -9
- Misses 24356 24359 +3
+ Partials 291 288 -3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pache/eventmesh/common/protocol/tcp/UserAgent.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL2NvbW1vbi9wcm90b2NvbC90Y3AvVXNlckFnZW50LmphdmE=) | `2.70% <0.00%> (+0.13%)` | :arrow_up: |
| [...tandalone/broker/task/HistoryMessageClearTask.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLWNvbm5lY3Rvci1wbHVnaW4vZXZlbnRtZXNoLWNvbm5lY3Rvci1zdGFuZGFsb25lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvY29ubmVjdG9yL3N0YW5kYWxvbmUvYnJva2VyL3Rhc2svSGlzdG9yeU1lc3NhZ2VDbGVhclRhc2suamF2YQ==) | `29.41% <0.00%> (-17.65%)` | :arrow_down: |
| [...mesh/connector/standalone/broker/MessageQueue.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLWNvbm5lY3Rvci1wbHVnaW4vZXZlbnRtZXNoLWNvbm5lY3Rvci1zdGFuZGFsb25lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvY29ubmVjdG9yL3N0YW5kYWxvbmUvYnJva2VyL01lc3NhZ2VRdWV1ZS5qYXZh) | `32.46% <0.00%> (-7.80%)` | :arrow_down: |
| [...rg/apache/eventmesh/runtime/trace/LogExporter.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL3RyYWNlL0xvZ0V4cG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ime/admin/handler/RedirectClientByPathHandler.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2FkbWluL2hhbmRsZXIvUmVkaXJlY3RDbGllbnRCeVBhdGhIYW5kbGVyLmphdmE=) | `84.78% <0.00%> (ø)` | |
| [...e/admin/handler/RedirectClientByIpPortHandler.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2FkbWluL2hhbmRsZXIvUmVkaXJlY3RDbGllbnRCeUlwUG9ydEhhbmRsZXIuamF2YQ==) | `31.37% <0.00%> (ø)` | |
| [...l/tcp/client/recommend/EventMeshRecommendImpl.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvdGNwL2NsaWVudC9yZWNvbW1lbmQvRXZlbnRNZXNoUmVjb21tZW5kSW1wbC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../tcp/client/session/push/DownStreamMsgContext.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvdGNwL2NsaWVudC9zZXNzaW9uL3B1c2gvRG93blN0cmVhbU1zZ0NvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../trace/pinpoint/exporter/PinpointSpanExporter.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/2550/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXRyYWNlLXBsdWdpbi9ldmVudG1lc2gtdHJhY2UtcGlucG9pbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC90cmFjZS9waW5wb2ludC9leHBvcnRlci9QaW5wb2ludFNwYW5FeHBvcnRlci5qYXZh) | `69.38% <0.00%> (+0.46%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045334987
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
1. The original code has no performance problems, and code style problems
2. Although the modified code looks more concise, it actually has some side effects. For example, a single line is too complex, the readability is poor, and the testability is poor.
If you want to know more details, it is recommended to read the google code review guide in depth.
https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] walterlife commented on a diff in pull request #2550: [Fixes #2537 ] Method stores return result in local
Posted by GitBox <gi...@apache.org>.
walterlife commented on code in PR #2550:
URL: https://github.com/apache/incubator-eventmesh/pull/2550#discussion_r1045189645
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/UserAgent.java:
##########
@@ -136,18 +136,17 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = subsystem != null ? subsystem.hashCode() : 0;
- result = 31 * result + (group != null ? group.hashCode() : 0);
- result = 31 * result + (path != null ? path.hashCode() : 0);
- result = 31 * result + pid;
- result = 31 * result + (host != null ? host.hashCode() : 0);
- result = 31 * result + (purpose != null ? purpose.hashCode() : 0);
- result = 31 * result + port;
- result = 31 * result + (version != null ? version.hashCode() : 0);
- result = 31 * result + (username != null ? username.hashCode() : 0);
- result = 31 * result + (password != null ? password.hashCode() : 0);
- result = 31 * result + (idc != null ? idc.hashCode() : 0);
- result = 31 * result + (env != null ? env.hashCode() : 0);
- result = 31 * result + unack;
- return result;
+ return (((((((((((result * 31 + (group != null ? group.hashCode() : 0))
Review Comment:
I don't think this way of writing is more readable now.
For example, there are too many parentheses, also not testable.
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org