optimize: Remove the unused defaultEventExecutorGroup #7414
Conversation
…in NettyClientBootstrap and make it final
| private final EventLoopGroup eventLoopGroupWorker; | ||
|
|
||
| private EventExecutorGroup defaultEventExecutorGroup; | ||
| private final EventExecutorGroup defaultEventExecutorGroup; |
There was a problem hiding this comment.
Was there any specific reason you decided not to remove defaultEventExecutorGroup?
There was a problem hiding this comment.
Was there any specific reason you decided not to remove
defaultEventExecutorGroup?
Considering the limited understanding of the original design, we will adhere to the principle of minimal changes and prioritize resolving the resource consumption issue with the DefaultEventExecutorGroup. If there are no plans to enable this feature in the future, I could also consider removing it.
There was a problem hiding this comment.
Was there any specific reason you decided not to remove
defaultEventExecutorGroup?Considering the limited understanding of the original design, we will adhere to the principle of minimal changes and prioritize resolving the resource consumption issue with the DefaultEventExecutorGroup. If there are no plans to enable this feature in the future, I could also consider removing it.
I think it can be deleted now.
There was a problem hiding this comment.
+1
Since we’ve already confirmed in the issue that it’s no longer being used, I believe it’s best to remove it!
Splitting the work into two steps introduces unnecessary process, in my opinion.
Since both PMC members and committers will review the code and catch any unexpected issues, I don’t think there’s any need to worry.
I believe this is the purpose of the code review process. 🙂
There was a problem hiding this comment.
It has been completely removed. Please review it, experts.😊
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #7414 +/- ##
============================================
+ Coverage 59.01% 59.02% +0.01%
+ Complexity 531 529 -2
============================================
Files 1281 1281
Lines 46109 46102 -7
Branches 5563 5562 -1
============================================
+ Hits 27210 27211 +1
+ Misses 16330 16327 -3
+ Partials 2569 2564 -5
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
funky-eyes
changed the title
|
Please send your DingTalk ID to |
3 participants
…make it final
Ⅰ. Describe what this PR did
Remove the unused defaultEventExecutorGroup initialization in NettyClientBootstrap and make it final
Ⅱ. Does this pull request fix one issue?
fixes #6132
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews