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

[Dependency Governance] Remove useless dependencies. #8472

Closed
CherishCai opened this issue May 28, 2022 · 9 comments
Closed

[Dependency Governance] Remove useless dependencies. #8472

CherishCai opened this issue May 28, 2022 · 9 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@CherishCai
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Too many useless dependencies, too many and confusing.
太多无用依赖,多且让人费解。

Describe the solution you'd like
Dependency Governance, remove useless dependencies.
依赖治理,去除无用依赖。

Describe alternatives you've considered
Perhaps many of the dependencies were initially added, and after optimization and upgrading, useless dependencies were not removed.
或许很多是初期加入的依赖,然后经过优化升级后,无用的依赖就没有被去除。

Additional context
mvn clean install -Dmaven.test.skip=true -P release-nacos
Make sure there are no compile errors.
务必已保障没有编译错误。

@KomachiSion
Copy link
Collaborator

请先在提交PR前在issue中列举无用依赖,
讨论后再提交PR。

@CherishCai
Copy link
Contributor Author

  1. 无用上的依赖 jcip-annotations
        <dependency>
            <groupId>net.jcip</groupId>
            <artifactId>jcip-annotations</artifactId>
            <optional>true</optional>
        </dependency>

            <dependency>
                <groupId>com.google.truth</groupId>
                <artifactId>truth</artifactId>
                <version>${truth.version}</version>
            </dependency>

            <!-- commons-logging nacos 项目自身无用,可由需要的三方依赖嵌套引入 -->
            <dependency>
                <groupId>commons-logging</groupId>
                <artifactId>commons-logging</artifactId>
                <version>${commons-logging.version}</version>
            </dependency>

            <!-- commons-dbcp 无用,用的是 HikariCP -->
            <dependency>
                <groupId>commons-dbcp</groupId>
                <artifactId>commons-dbcp</artifactId>
                <version>${commons-dbcp.version}</version>
            </dependency>

            <!-- commons-logging nacos 项目自身无用 -->
            <dependency>
                <groupId>commons-cli</groupId>
                <artifactId>commons-cli</artifactId>
                <version>${commons-cli.version}</version>
            </dependency>

image
image

  1. aop 可以由 spring-aop 统一依赖
        <!-- for aop -->
        <dependency>
            <groupId>org.aspectj</groupId>
            <artifactId>aspectjrt</artifactId>
        </dependency>
        <dependency>
            <groupId>cglib</groupId>
            <artifactId>cglib-nodep</artifactId>
        </dependency>
  1. no jsp, all rest api, delete tomcat-embed-jasper
            <dependency>
                <groupId>org.apache.tomcat.embed</groupId>
                <artifactId>tomcat-embed-jasper</artifactId>
                <version>${tomcat-embed-jasper.version}</version>
            </dependency>
  1. hamcrest 可以由 spring-boot-stater-test 统一依赖
    image

@CherishCai
Copy link
Contributor Author

  1. grpc-netty-shaded already shaded netty for grpc. nacos-api 也不需要它。nacos 项目自身已经没有直接使用 netty。

image

@KomachiSion
Copy link
Collaborator

jcip-annotations

这个应该可以删,可能是以前有引入过ThreadSafe或者NotThreadSafe, 移除掉编译不报错就行。

commons-dbcp

可以删, 以前用的dbcp,1.3以后换成hikari了

truth

应该可以删, 测试框架应该是Junit4

aop

可以删

jsp

可以删


commons-logging和commons-cli

这个不确定,可以再讨论下


hamcrest

这个不能删,api,common,client没有依赖spring,还需要依赖

@CherishCai
Copy link
Contributor Author

commons-logging和commons-cli
这个不确定,可以再讨论下

commons-cli 确实没有被使用,commons-logging 会被其他用到的地方嵌套依赖进来,nacos 获取日志都是 Slf4j 门面 api。


hamcrest
这个不能删,api,common,client没有依赖spring,还需要依赖

这个晚点我回滚下,会和 netty 依赖那个提交冲突 pom.xml,所以等那个 PR 合并了我再 rebase 同时回滚 hamcrest 的变动。

@CherishCai
Copy link
Contributor Author

不好意思,重新看了下依赖情况。hamcrest 会被 junit 嵌套依赖,所以是不需要指定的

以下是 api 模块的依赖图,主 pom.xml 去掉 hamcrest 是没问题的。

image

image

@KomachiSion
Copy link
Collaborator

不好意思,重新看了下依赖情况。hamcrest 会被 junit 嵌套依赖,所以是不需要指定的

以下是 api 模块的依赖图,主 pom.xml 去掉 hamcrest 是没问题的。

image

image

ok

@KomachiSion
Copy link
Collaborator

commons-logging和commons-cli
这个不确定,可以再讨论下

commons-cli 确实没有被使用,commons-logging 会被其他用到的地方嵌套依赖进来,nacos 获取日志都是 Slf4j 门面 api。

hamcrest
这个不能删,api,common,client没有依赖spring,还需要依赖

这个晚点我回滚下,会和 netty 依赖那个提交冲突 pom.xml,所以等那个 PR 合并了我再 rebase 同时回滚 hamcrest 的变动。

commons-logging改成optinal+provided吧,观察几个版本看看

@CherishCai
Copy link
Contributor Author

commons-logging改成optinal+provided吧,观察几个版本看看

其实,是不用的。也 cherry-pick 此提交内部发布了,它会被其他三方依赖嵌套引入。
image

image

@KomachiSion KomachiSion added dependencies Pull requests that update a dependency file and removed status/need feedback labels Jun 13, 2022
CherishCai added a commit to CherishCai/nacos that referenced this issue Jun 15, 2022
CherishCai added a commit to CherishCai/nacos that referenced this issue Jun 15, 2022
KomachiSion pushed a commit that referenced this issue Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

2 participants