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

slf4j-api in httpcomponets-testing module is set to a wrong dependency scope. #500

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

HappyHacker123
Copy link
Contributor

Problem Description

The slf4j-api is declared without assigning a dependency scope, leading to a default scope compile. But it's only used for testing with classes [org.slf4j.Logger, org.slf4j.LoggerFactory] in file “TypeAsyncResources.java" and "TestClientResources.java".

    <dependency>
      <groupId>org.slf4j</groupId>
      <artifactId>slf4j-api</artifactId>
    </dependency>

Possible Outcome

This brings redundant compile time dependency for the module, increasing the probability of dependency conflict for downstream users.

Solution

Changing the dependency scope to test can help. After changing the dependency scope to "test," I recompiled the module and executed the tests. Following these adjustments, the module compiled successfully, and all the tests passed.

@HappyHacker123
Copy link
Contributor Author

@ok2c @arturobernalg Could you look at these changes? Thx :p

@Sharofiddin
Copy link

This project by itself for testing, I think this change is redundant.

@HappyHacker123
Copy link
Contributor Author

This project by itself for testing, I think this change is redundant.

Thanks for your reply :) .

However, I think how downstream users use this project wouldn't hinder keeping a clean dependency tree. As maven official document goes, dependencies used for tests should be in test scope. Setting slf4j-api to test scope seems to do no harm.

What's more, declaring a dependency as "test" scope means that the dependency won't be transitive according to maven doc, which means that this dependency won't be propogated to downstream users that use this project, even if the project is used for test by downstream projects. Scope "compile" is just the other way round.

@ok2c
Copy link
Member

ok2c commented Nov 2, 2023

@HappyHacker123 httpclient5-testing depends on httpclient5 that will drag in slf4j-api as a transitive dependency with compile scope no matter what. I agree though there is no harm in making the scope explicit even it has no effect on the real dependency graph.

@ok2c ok2c merged commit f78c4ff into apache:master Nov 2, 2023
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.

3 participants