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: reduce unnecessary dependences in client side #1688

Merged
merged 2 commits into from
Feb 5, 2020

Conversation

ggndnn
Copy link
Contributor

@ggndnn ggndnn commented Sep 20, 2019

Ⅰ. Describe what this PR did

There are some unnecessary transitive dependencies of client side artifacts, e.g. rm-datasource, that may lead to dependent artifact conflict in user's environment.

...
<dependency>
    <groupId>com.esotericsoftware</groupId>
    <artifactId>kryo</artifactId>
</dependency>
<dependency>
    <groupId>de.javakaffee</groupId>
    <artifactId>kryo-serializers</artifactId>
</dependency>
...

This PR tried to remove unnecessary transitive dependencies of client side artifacts from pom.xml as much as possible.

Please have a review to check whether these changes are appropriate.

Ⅱ. 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

@xingfudeshi xingfudeshi self-requested a review September 21, 2019 06:51
@slievrly
Copy link
Member

if user use seata-all dependency, it will not contains provided scope dependency in all.pom.
image
image

@ggndnn
Copy link
Contributor Author

ggndnn commented Sep 23, 2019

It's ok to use seata-all as dependency. But if user use rm-datasource directly, some dependencies are unnecessary, which should be optional.

@codecov-io
Copy link

codecov-io commented Oct 11, 2019

Codecov Report

Merging #1688 into develop will increase coverage by 0.01%.
The diff coverage is 39.17%.

Impacted file tree graph

@@             Coverage Diff              @@
##             develop   #1688      +/-   ##
============================================
+ Coverage      52.78%   52.8%   +0.01%     
- Complexity      2499    2520      +21     
============================================
  Files            485     488       +3     
  Lines          15299   15341      +42     
  Branches        1754    1749       -5     
============================================
+ Hits            8076    8101      +25     
- Misses          6446    6466      +20     
+ Partials         777     774       -3
Impacted Files Coverage Δ Complexity Δ
...in/java/io/seata/server/event/EventBusManager.java 66.66% <ø> (ø) 1 <0> (ø) ⬇️
.../io/seata/common/loader/EnhancedServiceLoader.java 61.34% <0%> (-0.52%) 25 <0> (ø)
...in/java/io/seata/server/session/SessionHelper.java 64.86% <0%> (-15.14%) 8 <0> (ø)
...o/seata/server/coordinator/DefaultCoordinator.java 54.87% <100%> (+5.06%) 29 <0> (-1) ⬇️
.../java/io/seata/server/transaction/tcc/TccCore.java 100% <100%> (ø) 2 <2> (?)
...ava/io/seata/server/transaction/saga/SagaCore.java 2.7% <2.7%> (ø) 2 <2> (?)
...in/java/io/seata/server/session/BranchSession.java 78.1% <28.57%> (-2.35%) 44 <2> (ø)
...in/java/io/seata/server/transaction/at/ATCore.java 36.36% <36.36%> (ø) 3 <3> (?)
...java/io/seata/server/coordinator/AbstractCore.java 55.95% <55.95%> (ø) 12 <12> (?)
.../java/io/seata/server/coordinator/DefaultCore.java 55.27% <68.11%> (+15.69%) 23 <18> (-3) ⬇️
... and 7 more

Copy link
Member

@xingfudeshi xingfudeshi 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
Copy link
Contributor

zjinlei commented Jan 21, 2020

please resolve conflicts

@zjinlei zjinlei added this to the 1.1.0 milestone Jan 21, 2020
@zjinlei zjinlei added the status: merge-conflict Category prs with merge conflict. label Jan 24, 2020
@ggndnn
Copy link
Contributor Author

ggndnn commented Feb 4, 2020

please resolve conflicts
fixed

@zjinlei zjinlei removed the status: merge-conflict Category prs with merge conflict. label Feb 4, 2020
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

@slievrly slievrly merged commit d364751 into apache:develop Feb 5, 2020
@ggndnn ggndnn deleted the optimize_dependencies branch February 5, 2020 08:03
booogu pushed a commit to booogu/seata that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants