Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Clean groovy dependency and drive camel-quarkus from quarkus bom #1191

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Clean groovy dependency and drive camel-quarkus from quarkus bom #1191

merged 1 commit into from
Mar 26, 2024

Conversation

claudio4j
Copy link
Contributor

  • the groovy deps were excluded from rest-assured deps, but that's not the case anymore
  • This PR removes the maven property camel-quarkus-version, while the property named in the camel-k-catalog is camel-quarkus.version, the test in the groovy script verifies it.

Release Note

NONE

@claudio4j
Clean groovy dependency and drive camel-quarkus from quarkus bom
2f9769e 
- the groovy deps were excluded from rest-assured deps, but that's not the case anymore
- removing AvailablePortFinder class as it's not required anymore since camel-k-knative was removed
@claudio4j claudio4j requested a review from squakez March 22, 2024 15:37
squakez
squakez reviewed Mar 25, 2024
View reviewed changes
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't know how this would affect the future dependency which is on Camel Quarkus. I cannot give a meaningful review on this. Please, check with @lburgazzoli what he thinks about this change.

lburgazzoli
lburgazzoli reviewed Mar 25, 2024
View reviewed changes
support/camel-k-test/pom.xml
@@ -48,36 +48,11 @@
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<exclusions>
<exclusion>
<groupId>org.apache.groovy</groupId>
<artifactId>*</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is quarkus now enforcing the groovy version ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The org.apache.groovy:groovy is driven by io.quarkus.platform:quarkus-camel-bom which currently is the same version as defined in camel-quarkus bom. You can see by executing mvn dependency:tree

@lburgazzoli
Copy link
Contributor

I don't know what the sync_cs.sh does, for the other part LGTM

@claudio4j
Copy link
Contributor Author

More context about the changes:

  1. in support/camel-k-maven-plugin/pom.xml the dependency org.apache.camel.quarkus:camel-quarkus-catalog sets the version to ${camel-quarkus-version}, which may eventually be different from the version driven by quarkus-camel-bom, if there is a patch release from either projects. For the recently upstream version, this may not be a problem, but once we reach productization, this could lead to a different camel-quarkus-catalog version from the one defined in the quarkus-camel-bom. So, this change aligns the camel-quarkus-catalog version to the quarkus-camel-bom for consistency.

  2. The removal of the groovy dependencies from the test module, it brings the org.apache.groovy:groovy defined in the quarkus-camel-bom, which camel-quarkus relies on. For upstream this may not be a problem, but for downstream it may lead to different groovy version in the mrrc repository which eventually have vulnerable artifacts picked up by CVE reports. So, this is good to align to the quarkus-camel-bom.

@claudio4j
Copy link
Contributor Author

clear to merge ?

@claudio4j claudio4j merged commit f94d084 into apache:main Mar 26, 2024
22 checks passed
@claudio4j claudio4j deleted the upstream/main_groovy_dep branch March 27, 2024 10:33
claudio4j added a commit to claudio4j/camel-k-runtime that referenced this pull request May 7, 2024
@claudio4j
Clean groovy dependency and drive camel-quarkus from quarkus bom (apa…
b0b2f61 
…che#1191)

- the groovy deps were excluded from rest-assured deps, but that's not the case anymore
- removing AvailablePortFinder class as it's not required anymore since camel-k-knative was removed

(cherry picked from commit f94d084)
claudio4j added a commit to claudio4j/camel-k-runtime that referenced this pull request May 8, 2024
@claudio4j
Clean groovy dependency and drive camel-quarkus from quarkus bom (apa…
42be382 
…che#1191)

- the groovy deps were excluded from rest-assured deps, but that's not the case anymore
- removing AvailablePortFinder class as it's not required anymore since camel-k-knative was removed

(cherry picked from commit f94d084)
claudio4j added a commit that referenced this pull request May 8, 2024
@claudio4j
Backport changes from main to 3.8.x (#1207)
45305a5 
* Clean groovy dependency and drive camel-quarkus from quarkus bom (#1191)

- the groovy deps were excluded from rest-assured deps, but that's not the case anymore
- removing AvailablePortFinder class as it's not required anymore since camel-k-knative was removed

(cherry picked from commit f94d084)

* Remove tracing capability (#1204)

the camel-quarkus-opentracing was removed from camel-quarkus 3.0

(cherry picked from commit 5308e09)

* Remove unused camel-cloudevents dependency

* Remove runnerParentFirstArtifacts from quarkus-extension-maven-plugin
it seems not required anymore by examing the plugin doc
claudio4j added a commit to jboss-fuse/camel-k-runtime that referenced this pull request May 8, 2024
@claudio4j
Backport changes from main to 3.8.x (apache#1207)
2d795fd 
* Clean groovy dependency and drive camel-quarkus from quarkus bom (apache#1191)

- the groovy deps were excluded from rest-assured deps, but that's not the case anymore
- removing AvailablePortFinder class as it's not required anymore since camel-k-knative was removed

(cherry picked from commit f94d084)

* Remove tracing capability (apache#1204)

the camel-quarkus-opentracing was removed from camel-quarkus 3.0

(cherry picked from commit 5308e09)

* Remove unused camel-cloudevents dependency

* Remove runnerParentFirstArtifacts from quarkus-extension-maven-plugin
it seems not required anymore by examing the plugin doc

Conflict fixed manually

(cherry picked from commit apache/camel-k-runtime@45305a589)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lburgazzoli lburgazzoli lburgazzoli left review comments

@squakez squakez squakez left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@claudio4j @lburgazzoli @squakez