Why have we been adding annotations that are part of the public API to impl module?

[k-wall]

We've been adding our annotations that drive the Junit5 extension to io.kroxylicious.testing.kafka.common within the impl module. That seems like a surprising choice.

I think they ought to be in the testing-junit5-extension module, and probably under the io.kroxylicious.testing.kafka.junit5ext package.

[k-wall]

@SamBarker @franvila what are your thoughts?

[SamBarker]

I agree it seems odd to ask users to annotate their code with something from an impl module when we offer an api module as well.

It looks like the split originates from @tombentley initial work. @tombentley was there some specific thinking behind the @BrokerCluster being in the impl module rather than the API? I have hazy memories of deciding that @BrokerCluster was a concrete thing rather than a meta(?) thing like @KafkaClusterConstraint, but in hind sight that seems like a poor rationalisation.

[k-wall]

I agree it seems odd to ask users to annotate their code with something from an impl module when we offer an api module as well.

To be clear, it is not the api module I'm proposing as the target. That has no knowledge of the Junit5 extension. It is the junit5-extension. It already has some annotations such as @Name and @ConstraintsMethodSource. I am suggesting moving annotations such as @BrokerConfig to it.

[SamBarker]

The extension has an api module (doesn't it? Or is it just a package?) that's what I was referring too

[k-wall]

I think there are some wires crossed.

To be clear, it is annotations such as @BrokerConfig (package io.kroxylicious.testing.kafka.common) in the impl module that I see as odd. I am proposing moving these to the io.kroxylicious.testing.kafka.junit5ext package within the
junit5-extension module. That package already has annotations such as @Name.

The api module has no knowledge of the junit5 extension except for the @KafkaClusterConstraint (which KafkaClusterProvisioningStrategy uses to find the constraint annotations programmatically). I think this is okay.

[SamBarker]

I think there are some wires crossed.

To be clear, it is annotations such as @BrokerConfig (package io.kroxylicious.testing.kafka.common) in the impl module that I see as odd. I am proposing moving these to the io.kroxylicious.testing.kafka.junit5ext package within the junit5-extension module. That package already has annotations such as @Name.

The api module has no knowledge of the junit5 extension except for the @KafkaClusterConstraint (which KafkaClusterProvisioningStrategy uses to find the constraint annotations programmatically). I think this is okay.

Ok. In that case I'm happy with your proposal.

[k-wall]

I decided not to make this change right now. It turned out to be slightly harder than I anticipated. You end up with cyclic dependencies which I could not see a simple way to resolve.

I think we need a deeper reorganisation but don't want to devote the cycles to it right now.