tiagobento commented on PR #6772: URL: https://github.com/apache/incubator-kie-drools/pull/6772#issuecomment-4894216290
Hey @tkobayas, thanks a lot for taking the time to actually improve the dependency graph logic in the partial build mechanism! I have to say I have seen so many errors happen due to this convention of Quarkus'. I'm not sure why they chose to break the configuration-over-convention aspect of Maven itself, it is truly unpredictable. Your PR looks great, and I approve it. Just one minor comment: we have in some places specific dependency declarations that aim to solve that problem. E.g.,: - https://github.com/apache/incubator-kie-drools/blob/main/drools-quarkus-extension/drools-quarkus-examples/drools-quarkus-examples-reactive/pom.xml#L95-L112 - https://github.com/apache/incubator-kie-drools/blob/main/drools-quarkus-extension/drools-quarkus-examples/drools-quarkus-examples-multiunit/pom.xml#L68-L85 I think those two strategies are complimentary to each other, since this PR makes topology sorting include the `-deployment` modules, while these explicit dependency declarations in the POM put the `-deployment` modules in the classpath. But I guess it is true that explicitly declaring the `-deployment` modules makes this PR not have a lot of effect anymore, as normal dependencies are already correctly picked up by the current dependency graph logic. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
