-
Notifications
You must be signed in to change notification settings - Fork 26.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change main method under dubbo-remoting-api to standard test #5026
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. expect that for loop. maybe remove it if it's not necessary for unit test
System.out.println("handle:" + msg + ";thread:" + Thread.currentThread().getName()); | ||
return new StringMessage("hello world"); | ||
dispatcher.addReplier(Data.class, (channel, msg) -> { | ||
for (int i = 0; i < 10000; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is existing code. but I'm thinking what's purpose for this code? looks like this is not necessary for this unit test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this loop seems to be unnecessary, just to keep the existing code. I have changed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. will merge it once CI pass
Codecov Report
@@ Coverage Diff @@
## master #5026 +/- ##
============================================
- Coverage 63.99% 63.93% -0.07%
- Complexity 451 453 +2
============================================
Files 769 769
Lines 33176 33194 +18
Branches 5230 5236 +6
============================================
- Hits 21231 21222 -9
- Misses 9523 9551 +28
+ Partials 2422 2421 -1
Continue to review full report at Codecov.
|
Thanks for your contribution |
Because of the commits linked to the wrong user, I resubmitted a pr and continued #4957 @htynkn