Skip to content

Conversation

@rschmitt
Copy link
Contributor

No description provided.

@ok2c
Copy link
Member

ok2c commented May 23, 2025

@rschmitt I never found Maven wrapper to be very useful, but I might have never had a good use cases for it. Please just do the same in core

@garydgregory
Copy link
Member

Almost -1: I avoid these wrappers myself since they tend to go stale and no one maintains them. They are confusing at best. I also never want to have to remember what directory I need to type "mvn" vs whatever the wrapper is called and I certainly don't want have it force some version of Maven on me, especially by downloading it. I also don't want to update any of my scripts or rewrite instructions on the site and wiki, which this PR doesn't do and demonstrates one of the problems. The PR description is empty and the reader file doesn't even describe when or why I need this thing, or if it's a requirement or what.

Copy link
Member

@arturobernalg arturobernalg left a comment

Choose a reason for hiding this comment

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

@rschmitt I'm generally not a fan of Maven Wrapper because it often goes stale and adds unnecessary complexity for little benefit.

@rschmitt
Copy link
Contributor Author

I'm generally not a fan of Maven Wrapper because it often goes stale and adds unnecessary complexity for little benefit.

I'm surprised by this. The Maven wrapper is quite convenient. For instance, my Linux desktop doesn't actually have a version of Maven recent enough to build HttpComponents, and by far the easiest way to obtain one was to set up the wrapper. I also invoke the wrapper from #635 to help ensure that the example runner just works.

I think it would be a good idea in principle to use the Maven wrapper in the GitHub workflows, but it's not a huge difference either way since Maven is fairly stable. (With a Gradle project, by contrast, using the wrapper would be absolutely non-negotiable.)

@ok2c
Copy link
Member

ok2c commented May 24, 2025

@rschmitt Gradle is a whole different story. I see no harm in having Maven wrapper. It may even become useful if we decide to upgrade to Maven 4. Just please apply the same config to core as well.

@rschmitt rschmitt merged commit 7e64580 into apache:master May 24, 2025
10 checks passed
@rschmitt rschmitt deleted the mvnw branch May 26, 2025 00:27
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.

4 participants