Skip to content
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

optimize: optimize exceptionHandler code logic #2489

Merged
merged 4 commits into from
Apr 12, 2020

Conversation

YuKongEr
Copy link
Contributor

@YuKongEr YuKongEr commented Apr 1, 2020

Ⅰ. Describe what this PR did

Optimizi code logic

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@objcoding
Copy link
Contributor

This part of the code has been refactored in #2486

@l81893521 l81893521 added the first-time contributor first-time contributor label Apr 1, 2020
@codecov-io
Copy link

codecov-io commented Apr 1, 2020

Codecov Report

Merging #2489 into develop will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2489      +/-   ##
=============================================
+ Coverage      51.06%   51.09%   +0.02%     
- Complexity      2765     2767       +2     
=============================================
  Files            550      550              
  Lines          17529    17529              
  Branches        2064     2064              
=============================================
+ Hits            8951     8956       +5     
+ Misses          7735     7733       -2     
+ Partials         843      840       -3     
Impacted Files Coverage Δ Complexity Δ
...seata/core/exception/AbstractExceptionHandler.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...in/java/io/seata/server/session/GlobalSession.java 85.32% <0.00%> (+0.45%) 72.00% <0.00%> (+1.00%)
...o/seata/server/coordinator/DefaultCoordinator.java 55.00% <0.00%> (+0.50%) 29.00% <0.00%> (+1.00%)
...torage/file/store/FileTransactionStoreManager.java 57.46% <0.00%> (+0.95%) 29.00% <0.00%> (ø%)

@slievrly slievrly added the status: waiting-for-feedback Waiting for feedback label Apr 3, 2020
@YuKongEr
Copy link
Contributor Author

YuKongEr commented Apr 4, 2020

ok i think AbstractTransactionRequest#exceptionHandleTemplate() use generic method would be better

@objcoding
Copy link
Contributor

ok i think AbstractTransactionRequest#exceptionHandleTemplate() use generic method would be better

What you said should be AbstractExceptionHandler#exceptionHandleTemplate.

Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. AbstractExceptionHandler#exceptionHandleTemplate: this optimization is ok.
    2.AbstractRpcRemoting: this part of the code has refactored in PR-2486, maybe you can participate in the code review of PR-2486.

Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@zjinlei zjinlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zjinlei zjinlei added this to the 1.2.0 milestone Apr 10, 2020
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slievrly slievrly changed the title Optimizi code logic optimize: optimize exceptionTemplate code logic Apr 12, 2020
@slievrly slievrly changed the title optimize: optimize exceptionTemplate code logic optimize: optimize exceptionHandler code logic Apr 12, 2020
@slievrly
Copy link
Member

@YuKongEr Thank you for your contribution.

@slievrly slievrly removed the status: waiting-for-feedback Waiting for feedback label Apr 12, 2020
@slievrly slievrly merged commit e32a95c into apache:develop Apr 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants