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.

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

Fix for WFMP-209, Can't use maven http transport with maven < 3.9 #343

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

jfdenise
Copy link
Contributor

@jfdenise jfdenise commented May 30, 2023

Issue: https://issues.redhat.com/browse/WFMP-209

The maven-resolver dependencies are not needed by the plugin. We must rely on default Maven handling of the transport aspects.

@jfdenise jfdenise requested a review from jamezp as a code owner May 30, 2023 10:22
@jfdenise jfdenise force-pushed the main branch 2 times, most recently from ae3fde1 to 5761661 Compare May 30, 2023 14:22
@jfdenise
Copy link
Contributor Author

@jamezp , the remaining failure is un-related to this change. This build image tests failed in some previous runs.

@jamezp
Copy link
Member

jamezp commented Jun 1, 2023

@jamezp , the remaining failure is un-related to this change. This build image tests failed in some previous runs.

Yes, I've been thinking about disabling these tests for Windows as they seem flaky. I guess I should dig into why they are flaky first though :)

@@ -61,17 +61,17 @@
<dependency>
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-api</artifactId>
<scope>test</scope>
<scope>provided</scope>
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for these being changed from test to provided? TBH it looks weird to me to have them at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are required for compilation, they were transitively included.

Copy link
Member

Choose a reason for hiding this comment

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

There isn't anything that's a non-test that should need to be compiled. If test scope doesn't work it seems like something is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jamezp you were right, I fixed the pom.

@jamezp jamezp merged commit 5129133 into wildfly:main Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants