MINOR: Use testFixturesImplementation instead of testFixturesApi#22243
Conversation
github-actions
Bot
added
triage
|
|
||
| testFixturesApi project(':test-common:test-common-util') | ||
| testFixturesImplementation project(':test-common:test-common-util') | ||
| testFixturesApi libs.bcpkix |
There was a problem hiding this comment.
Should we apply it to other parts as well?
There was a problem hiding this comment.
No, the remaining four declarations should all stay as testFixturesApi — all of them expose library types in public method signatures of the test fixtures, so consumers need them transitively.
There was a problem hiding this comment.
The api declaration will transfer the dependencies publicly, and I think that is a anti-pattern
There was a problem hiding this comment.
I can see logic behind carrying the dependencies forward if we know they’ll always be necessary in the dependent modules. But if we don’t want to do this the downside seems low, dependent modules will have to directly specify these. There’s advantages in being more explicit too.
m1a2st
force-pushed
the
MINOR-use-testFixturesImplementation
branch
from
1d0da97 to
93a8e10
Compare
| testFixturesImplementation project(':test-common:test-common-util') | ||
| testFixturesImplementation libs.bcpkix | ||
| testFixturesImplementation libs.junitJupiter | ||
| testFixturesImplementation libs.jqwik |
There was a problem hiding this comment.
It seems jqwik doesn't need to be in testImplementation here, as ArbitraryMemoryRecords is only used by the raft module. Should we move ArbitraryMemoryRecords to the raft module instead?
3 participants
See https://github.com/apache/kafka/pull/22201/files#r3212511936
Reviewers: Chia-Ping Tsai chia7712@gmail.com, Igor Soarez
i@soarez.me