-
Notifications
You must be signed in to change notification settings - Fork 188
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
Clean up camel-k extension leftovers #5802
Conversation
lburgazzoli
commented
Feb 26, 2024
- interceptors are not more needed
- customizers are not more needed
- custom source loader not more needed and replaced with camel.main.routes-include-pattern
- custom "Runtime" not more needed
faaf778
to
2679839
Compare
Any idea if these Groovy bits are still required (I'm assuming they are for Groovy support)? https://github.com/apache/camel-quarkus/blob/main/poms/bom/pom.xml#L6599-L6623 Also is this test related stuff is still needed? https://github.com/apache/camel-quarkus/blob/main/poms/bom-test/pom.xml#L135-L144 |
2679839
to
6ef78ba
Compare
Removed, let see |
6ef78ba
to
25d0b96
Compare
- interceptors are not more needed - customizers are not more needed - custom source loader not more needed and replaced with camel.main.routes-include-pattern - custom "Runtime" not more needed
25d0b96
to
0aab5ec
Compare
it looks like removing them does not cause any issue. Since the camel-k now leverages camel-groovy from core/quarkus, I guess they are not more needed.
Those were not needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left a couple of comments for some minor things to fix up.
<description>Integration tests for Camel Quarkus K Runtime Shutdown extension</description> | ||
|
||
<properties> | ||
<quarkus.runner.jar>${project.build.directory}/quarkus-app/quarkus-run.jar</quarkus.runner.jar> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the runnable JAR is never packaged in the platform build, we'll need to exclude these tests there.
You can exclude the test module here:
https://github.com/apache/camel-quarkus/blob/main/tooling/test-list/pom.xml#L57
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
</dependency> | ||
<dependency> | ||
<groupId>org.apache.camel.quarkus</groupId> | ||
<artifactId>camel-quarkus-platform-http-deployment</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you can remove platform-http & rest from this list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, also removed other deps not needed
Hopefully the final question - is this still needed? https://github.com/apache/camel-quarkus/tree/main/tooling/camel-k-maven-plugin |
yes, at least for some time it will be still required hoping to find a better solutions and eventually get rid of it. |
0aab5ec
to
4142107
Compare