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

[MWRAPPER-129][MWRAPPER-128] Drop legacy stuff, require Maven 3.6.3+ #126

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Apr 16, 2024

Also refresh and reshuffle to not duplicate dependencies and versions. Tend to use properties for versions where there is 2+ of them, otherwise is just fluff.


https://issues.apache.org/jira/browse/MWRAPPER-128
https://issues.apache.org/jira/browse/MWRAPPER-129

Also refresh and reshuffle to not duplicate
dependencies and versions. Tend to use properties
for versions where there is 2+ of them, otherwise
is just fluff.

---

https://issues.apache.org/jira/browse/MWRAPPER-128
@cstamas cstamas self-assigned this Apr 16, 2024
Copy link
Contributor

@jorsol jorsol left a comment

Choose a reason for hiding this comment

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

The org.codehaus.plexus:plexus-io dependency seems that is not used.

@@ -34,7 +34,7 @@ under the License.
<description>The Maven Wrapper Plugin is a plugin that provides support for the Maven Wrapper by unpacking Maven Wrapper Distribution to the current project.</description>

<prerequisites>
<maven>${mavenVersion}</maven>
<maven>3.2.5</maven>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not 3.6.3? this PR is about dropping legacy stuff ;)

Copy link
Member Author

@cstamas cstamas Apr 16, 2024

Choose a reason for hiding this comment

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

Yeah, I did not want to change prerequisite, it was more to kill this antipattern to "compile against eons old" maven that prevents noticing what is being deprecated lately in APIs and Core... (and replaced eon old aether w/ maven-resolver)

Copy link
Member

Choose a reason for hiding this comment

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

I'm still not sure about it ... when we depend on newer version we can have some issues in runtime which is not cover by ITs.
But from other side we have more information about depractions ...

The perfect way will be for me depends on the same version as in in prerequisites ... but we block using plugin for older Maven ....

So we need a compromise on it 😄

Copy link
Member

Choose a reason for hiding this comment

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

but we should be consequential and put here 3.6.3 as we do in more of plugins now

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but all plugin builds use at least two Maven version: 3.9.6 and ... usually 3.6.3. So what "issue in runtime" you expect? Okay, if test coverage is poor... but then we fix and improve coverage, and life goes on...

maven-wrapper-plugin/pom.xml Outdated Show resolved Hide resolved
Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

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

Please also add to commit message and issue title, that we also require 3.6.3 to be clear in release notes

@cstamas cstamas changed the title [MWRAPPER-128] Drop legacy stuff [MWRAPPER-129][MWRAPPER-128] Drop legacy stuff Apr 16, 2024
@cstamas cstamas changed the title [MWRAPPER-129][MWRAPPER-128] Drop legacy stuff [MWRAPPER-129][MWRAPPER-128] Drop legacy stuff, require Maven 3.6.3+ Apr 16, 2024
@cstamas
Copy link
Member Author

cstamas commented Apr 16, 2024

Tomorrow will check is plexus-io really used or not....

@jorsol
Copy link
Contributor

jorsol commented Apr 17, 2024

Tomorrow will check is plexus-io really used or not....

Maybe is used transitively, so never mind.

@cstamas
Copy link
Member Author

cstamas commented Apr 17, 2024

Hm, removed it and it builds...

@cstamas
Copy link
Member Author

cstamas commented Apr 17, 2024

Yes, is used:

[INFO] --- toolbox:0.1.3:tree (default-cli) @ maven-wrapper-plugin ---
[INFO] org.apache.maven.plugins:maven-wrapper-plugin:jar:3.2.1-SNAPSHOT
[INFO] +- javax.inject:javax.inject:jar:1 [compile]
[INFO] +- org.codehaus.plexus:plexus-archiver:jar:4.9.2 [compile]
[INFO] |  +- org.codehaus.plexus:plexus-io:jar:3.4.2 [compile]
[INFO] |  +- org.apache.commons:commons-compress:jar:1.26.1 [compile]
[INFO] |  +- commons-codec:commons-codec:jar:1.16.1 [compile]
[INFO] |  +- org.iq80.snappy:snappy:jar:0.4 [compile]
[INFO] |  +- org.tukaani:xz:jar:1.9 [runtime]
[INFO] |  \- com.github.luben:zstd-jni:jar:1.5.5-11 [runtime]
[INFO] +- commons-io:commons-io:jar:2.16.1 [compile]
[INFO] \- org.apache.maven.shared:maven-shared-utils:jar:3.4.2 [compile]

@cstamas cstamas merged commit 8e416f3 into master Apr 17, 2024
51 checks passed
@cstamas cstamas deleted the MWRAPPER-128 branch April 17, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants