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: add methods to reduce redundant code #2494

Merged
merged 5 commits into from
Apr 13, 2020
Merged

optimize: add methods to reduce redundant code #2494

merged 5 commits into from
Apr 13, 2020

Conversation

Zh1Cheung
Copy link
Contributor

Ⅰ. Describe what this PR did

  1. Reduced duplicate code in else branch (else statements in index> = 0 and else statements) and optimized code logic (judgment annotation.isParamInProperty () in advance) is more readable. In addition, there is still a large amount of code in this class. If else statement, this commit is only a further simplified code, but it is not optimal.
  2. I think if the code is extended in this class in the future, it will become more and more difficult to maintain (the fetchContextFromObject method has too much logic and high coupling)

Ⅱ. Does this pull request fix one issue?

xxx

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

xxx

Ⅳ. Describe how to verify it

xxx

Ⅴ. Special notes for reviews

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

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.

please format the code according to the p3c specification

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.

@slievrly
Copy link
Member

@Zh1Cheung please format the code you modify,there are some code style check not passed.

@zjinlei zjinlei added this to the 1.2.0 milestone Apr 10, 2020
@Zh1Cheung
Copy link
Contributor Author

@Zh1Cheung please format the code you modify,there are some code style check not passed.
Sorry,I encountered the same problem as #2510 , and I can't find the result.xml file he said.And it will report Non-resolvable import POM: Could not find artifact io.seata: seata-bom: pom: 1.2.0-SNAPSHOT @ io.seata: seata-parent: $ {revision}, seata \ pom.xml, line 164, column 25.I don't know how to solve it, thanks

@slievrly
Copy link
Member

@Zh1Cheung please format the code you modify,there are some code style check not passed.
Sorry,I encountered the same problem as #2510 , and I can't find the result.xml file he said.And it will report Non-resolvable import POM: Could not find artifact io.seata: seata-bom: pom: 1.2.0-SNAPSHOT @ io.seata: seata-parent: $ {revision}, seata \ pom.xml, line 164, column 25.I don't know how to solve it, thanks

remove the first blank line.

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

LGTM

@diguage
Copy link
Contributor

diguage commented Apr 13, 2020

@Zh1Cheung please format the code you modify,there are some code style check not passed.
Sorry,I encountered the same problem as #2510 , and I can't find the result.xml file he said.And it will report Non-resolvable import POM: Could not find artifact io.seata: seata-bom: pom: 1.2.0-SNAPSHOT @ io.seata: seata-parent: $ {revision}, seata \ pom.xml, line 164, column 25.I don't know how to solve it, thanks

I found the XML file. It locates at seata/compressor/seata-compressor-lz4/target/checkstyle-result.xml. So, the XML file should be at the /target/checkstyle-result.xml of the module.

#2510 (comment) is my reply.

@codecov-io
Copy link

codecov-io commented Apr 13, 2020

Codecov Report

Merging #2494 into develop will decrease coverage by 0.13%.
The diff coverage is 14.28%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2494      +/-   ##
=============================================
- Coverage      51.07%   50.93%   -0.14%     
+ Complexity      2766     2764       -2     
=============================================
  Files            550      550              
  Lines          17525    17521       -4     
  Branches        2063     2032      -31     
=============================================
- Hits            8951     8925      -26     
+ Misses          7734     7732       -2     
- Partials         840      864      +24     
Impacted Files Coverage Δ Complexity Δ
...io/seata/rm/tcc/interceptor/ActionContextUtil.java 62.06% <14.28%> (+7.52%) 5.00 <0.00> (ø)
...obuf/convertor/BranchRegisterRequestConvertor.java 90.47% <0.00%> (-9.53%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalBeginResponseConvertor.java 92.59% <0.00%> (-7.41%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalCommitRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalStatusRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...obuf/convertor/GlobalRollbackRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterTMRequestConvertor.java 94.73% <0.00%> (-5.27%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterRMRequestConvertor.java 90.47% <0.00%> (-4.77%) 3.00% <0.00%> (ø%)
...otobuf/convertor/BranchReportRequestConvertor.java 95.45% <0.00%> (-4.55%) 3.00% <0.00%> (ø%)
...tobuf/convertor/BranchReportResponseConvertor.java 95.45% <0.00%> (-4.55%) 3.00% <0.00%> (ø%)
... and 15 more

@Zh1Cheung
Copy link
Contributor Author

@Zh1Cheung please format the code you modify,there are some code style check not passed.
Sorry,I encountered the same problem as #2510 , and I can't find the result.xml file he said.And it will report Non-resolvable import POM: Could not find artifact io.seata: seata-bom: pom: 1.2.0-SNAPSHOT @ io.seata: seata-parent: $ {revision}, seata \ pom.xml, line 164, column 25.I don't know how to solve it, thanks

remove the first blank line.

Thanks,I ran xx before (at that time my first line was not a blank line), after the build was successful, I checked the result file and found no errors, just

<? xml version = "1.0" encoding = "UTF-8"?>
<checkstyle version = "6.18">
</ checkstyle>

Later, I have not noticed this problem (the first line at this time is a blank line), I always thought it was a problem of maven dependency

In addition, I want to know if I will continue to work under this pr or create a new pr when I want to work on code optimization ?

@Zh1Cheung
Copy link
Contributor Author

@Zh1Cheung please format the code you modify,there are some code style check not passed.
Sorry,I encountered the same problem as #2510 , and I can't find the result.xml file he said.And it will report Non-resolvable import POM: Could not find artifact io.seata: seata-bom: pom: 1.2.0-SNAPSHOT @ io.seata: seata-parent: $ {revision}, seata \ pom.xml, line 164, column 25.I don't know how to solve it, thanks

I found the XML file. It locates at seata/compressor/seata-compressor-lz4/target/checkstyle-result.xml. So, the XML file should be at the /target/checkstyle-result.xml of the module.

#2510 (comment) is my reply.

Thanks, I found this file later

@slievrly slievrly changed the title optimize:Add methods to reduce redundant code optimize: add methods to reduce redundant code Apr 13, 2020
@slievrly slievrly merged commit f810116 into apache:develop Apr 13, 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

8 participants