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

Tidy the dependency #981

Merged
merged 9 commits into from
May 7, 2019
Merged

Conversation

lovepoem
Copy link
Member

@lovepoem lovepoem commented May 6, 2019

Ⅰ. Describe what this PR did

tidy the maven dependency

Ⅱ. Does this pull request fix one issue?

#933

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

Config change

Ⅳ. Describe how to verify it

ci pass

Ⅴ. Special notes for reviews

1、Unify the version of netty
2、Remove guava ,use caffine to replace guava cache
3、Remove other useless jars
4、Upgrade junit4 to junit5
5、Remove testng (use junit5 )

2、Remove guava ,use [caffine](https://github.com/ben-manes/caffeine) to instead of guava cache
3、Remove other useless jars
4、Upgrade junit4 to [junit5](https://junit.org/junit5/docs/current/user-guide)
5、Remove testng
@codecov-io
Copy link

codecov-io commented May 6, 2019

Codecov Report

Merging #981 into develop will increase coverage by 1.36%.
The diff coverage is 37.5%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #981      +/-   ##
=============================================
+ Coverage      36.97%   38.34%   +1.36%     
- Complexity      1033     1046      +13     
=============================================
  Files            225      219       -6     
  Lines           8908     8714     -194     
  Branches        1080     1085       +5     
=============================================
+ Hits            3294     3341      +47     
+ Misses          5212     4951     -261     
- Partials         402      422      +20
Impacted Files Coverage Δ Complexity Δ
...datasource/undo/mysql/MySQLUndoUpdateExecutor.java 90.9% <ø> (ø) 6 <0> (ø) ⬇️
...java/io/seata/server/session/SessionCondition.java 66.66% <ø> (ø) 2 <0> (ø) ⬇️
...main/java/io/seata/core/rpc/netty/TmRpcClient.java 29.16% <ø> (ø) 12 <0> (ø) ⬇️
...n/java/io/seata/rm/datasource/ConnectionProxy.java 9.52% <ø> (-0.12%) 4 <0> (ø)
.../main/java/io/seata/rm/datasource/AsyncWorker.java 16.43% <ø> (ø) 4 <0> (ø) ⬇️
...java/io/seata/rm/datasource/DataSourceManager.java 28.33% <0%> (ø) 7 <0> (ø) ⬇️
...seata/rm/datasource/sql/struct/TableMetaCache.java 48.97% <60%> (-1.03%) 9 <2> (+1)
...ta/discovery/registry/FileRegistryServiceImpl.java 0% <0%> (-61.54%) 0% <0%> (-7%)
...a/io/seata/discovery/registry/RegistryFactory.java 0% <0%> (-50%) 0% <0%> (-2%)
...java/io/seata/discovery/registry/RegistryType.java 0% <0%> (-38.47%) 0% <0%> (-2%)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 054c879...e3d2769. Read the comment docs.

@xingfudeshi xingfudeshi self-requested a review May 6, 2019 09:59
@hongweiyi
Copy link
Contributor

Suggestion:

  1. make commons-pool2(just for redis) provided?
  2. upgrade FastJSON to 1.2.50, 1.2.48 has security issue

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.

@lovepoem
Copy link
Member Author

lovepoem commented May 7, 2019

Suggestion:
make commons-pool2(just for redis) provided?
upgrade FastJSON to 1.2.50, 1.2.48 has security issue

Done in a9a09ce

@xingfudeshi xingfudeshi self-requested a review May 7, 2019 02:31
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.

there are some conflict files.

@lovepoem
Copy link
Member Author

lovepoem commented May 7, 2019

there are some conflict files.

fixed

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.

great job,but I have some confusion,why delete seata-* in dependencyManagement, and each module has a <version>${project.version}</version> in other pom?

@lovepoem
Copy link
Member Author

lovepoem commented May 7, 2019

great job,but I have some confusion,why delete seata-* in dependencyManagement, and each module has a ${project.version} in other pom?

I think the version of dependecies we put in the dependencyManagement is changeable, but the version of submodule dependency is fixed, we can use ${project.version} , so I think there is a little redundancy to put all submodules in the seata/pom.xml

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

@lovepoem lovepoem merged commit 43fb0d3 into apache:develop May 7, 2019
@lovepoem lovepoem deleted the tidy_the_dependency branch May 7, 2019 08:36
nick-tan pushed a commit to nick-tan/seata that referenced this pull request Jul 12, 2019
tidy the maven dependency
* 1、Unify the version of `netty`
* 2、Use [caffine](https://github.com/ben-manes/caffeine) to instead of guava cache
* 3、Upgrade junit4 to [junit5](https://junit.org/junit5/docs/current/user-guide)
* 4、Remove testng (use [junit5](https://junit.org/junit5/docs/current/user-guide))
@wangliang181230 wangliang181230 added this to the 0.5.* milestone Aug 9, 2021
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

6 participants