Add support for Quarkus CXF product#1909
Conversation
| @@ -1,4 +1,4 @@ | |||
| package software.tnb.product.cq.configuration; | |||
| package software.tnb.product.quarkus.vanilla.configuration; | |||
There was a problem hiding this comment.
if you have separate packages for different variants, then imho does not make sense for the camel and cxf configurations to be within a class in quarkus.vanilla package
|
|
||
| @Override | ||
| public void customizeIntegrationBuilder(AbstractIntegrationBuilder<?> integrationBuilder) { | ||
| integrationBuilder.startupRegex(DEFAULT_STARTUP_REGEX); |
There was a problem hiding this comment.
this is equivalent to not calling the method at all, isnt it?
| public String getExtensions() { | ||
| // CXF Quarkus needs smallrye-health for proper readiness probes | ||
| String base = QuarkusVariant.super.getExtensions(); | ||
| return base.isEmpty() ? "smallrye-health" : base + ",smallrye-health"; |
There was a problem hiding this comment.
maybe change the method to List<String> (od maybe set? as the order is not important and we don't need to handle possible duplicates) so that you don't need to do this?
related to comment below
|
|
||
| @Override | ||
| public void customizeIntegrationBuilder(AbstractIntegrationBuilder<?> integrationBuilder) { | ||
| QuarkusVariant.super.customizeIntegrationBuilder(integrationBuilder); |
There was a problem hiding this comment.
since you need to call methods from parent class, wouldn't an abstract parent class be better fit instead of interface?
| /** | ||
| * Customizes integration builder. | ||
| */ | ||
| default void customizeIntegrationBuilder(AbstractIntegrationBuilder<?> integrationBuilder) { |
There was a problem hiding this comment.
since it is literally called customize, did you consider using customizers? would that work? then instead of calling variant.customizeIntegrationBuilder in LocalQuarkus call integrationBuilder.addCustomizers(variant.getCustomizers()) (this would need to be added, as there is no method signature for list/array of customizers)
| Dependency additionalBom = new Dependency(); | ||
| additionalBom.setGroupId(variant.additionalBomGroupId()); | ||
| additionalBom.setArtifactId(variant.additionalBomArtifactId()); | ||
| additionalBom.setVersion(variant.additionalBomVersion()); | ||
| additionalBom.setType("pom"); | ||
| additionalBom.setScope("import"); | ||
| model.getDependencyManagement().getDependencies().add(additionalBom); |
There was a problem hiding this comment.
would be cool if this was moved to a separate customizer (maybe together with extensions) as they do not really need to happen before all other customizers. you don't need to do it right now, but if you agree, could you open an issue for that? I think it could simplify the variants by just adding a customizer instead of implementing separate methods for g,a,v (but is related to the other comments related to variants) and extensions?
because when looking at the changes, it seems a little bit strange to pass the QuarkusVariant instance through 3 classes just to get the BOM and extensions
| /** | ||
| * Forces the given encoding for the output. | ||
| */ | ||
| public class QuarkusEncodingCustomizer extends CamelQuarkusCustomizer { |
There was a problem hiding this comment.
this extends a class from camel inside vanilla package.
I'd say this customizer should work for all quarkus flavors
There was a problem hiding this comment.
Thanks, so it should extends directly Customizer with no product in constructor?
There was a problem hiding this comment.
as i was thinking if i should create QuarkusCustomizer parent class, but we don't have PRODUCT Quarkus and i'm not sure if i should create it.
2 participants
No description provided.