-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
bugfix: fix encode request error #2456
Conversation
Signed-off-by: slievrly <slievrly@163.com>
Codecov Report
@@ Coverage Diff @@
## develop #2456 +/- ##
=============================================
- Coverage 51.45% 51.41% -0.04%
Complexity 2665 2665
=============================================
Files 529 529
Lines 16956 16975 +19
Branches 2051 2050 -1
=============================================
+ Hits 8724 8727 +3
- Misses 7408 7424 +16
Partials 824 824
|
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.
You can consider setting the parameter globalStatus of AbstractGlobalEndResponse to a constructor with parameters to prevent forgetting
@@ -303,4 +329,16 @@ public void execute(GlobalReportRequest request, GlobalReportResponse response) | |||
protected abstract void doGlobalReport(GlobalReportRequest request, GlobalReportResponse response, | |||
RpcContext rpcContext) throws TransactionException; | |||
|
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.
lack GlobalReportResponse.setGlobalStatus()
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.
fixed.
Signed-off-by: slievrly <slievrly@163.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.
LGTM
Agree with @ph3636
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
Signed-off-by: slievrly slievrly@163.com
Ⅰ. Describe what this PR did
bugfix: fix encode request error
Ⅱ. Does this pull request fix one issue?
fix #2458
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews