-
Notifications
You must be signed in to change notification settings - Fork 917
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
feat: deep copy the configuration when instanceOptions flow to ServerOptions/ClientOptions #2596
Conversation
…Options/ClientOptions Fixes: apache#2592 Signed-off-by: setcy <asetcy@gmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2596 +/- ##
==========================================
+ Coverage 47.55% 47.62% +0.06%
==========================================
Files 318 339 +21
Lines 23627 24925 +1298
==========================================
+ Hits 11236 11870 +634
- Misses 11328 11902 +574
- Partials 1063 1153 +90 ☔ View full report in Codecov by Sentry. |
Signed-off-by: setcy <asetcy@gmail.com>
Signed-off-by: setcy <asetcy@gmail.com>
Signed-off-by: setcy <asetcy@gmail.com>
Signed-off-by: setcy <asetcy@gmail.com>
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.
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.
Nice Job @setcy !
There are some points that we could discuss:
Clone
relys on handwriting individual fields. It is easy to miss corresponding clone procedure when we add fields. I think we should add test to ensure that the missing would not happen.- The issue had illustrated the modifying problem. Maybe we could add a test in dubbo_test.go to cover this scenario?(i.e. Modify some parts of configuration in Client/Server layer, and the configurations in upper layer did not be modified)
Signed-off-by: setcy <asetcy@gmail.com>
Quality Gate passedIssues Measures |
…Options/ClientOptions (apache#2596) fixes apache#2592
#2592