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

[MRESOLVER-365] Upgrade dependencies #294

Merged
merged 8 commits into from
Jun 2, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jun 1, 2023

@cstamas cstamas self-assigned this Jun 1, 2023
Update, and produce a bundle that simply needs to be unzipped,
drop all that site stuff that is hard and error prone to maintain.
As it is now misleading
@cstamas cstamas requested a review from michael-o June 1, 2023 09:12
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

Move the Redisson change to a separate issue because it requires testing

@cstamas
Copy link
Member Author

cstamas commented Jun 1, 2023

Done, #296

@cstamas cstamas requested a review from michael-o June 1, 2023 11:06
@gnodet gnodet added this to the 1.9.4 milestone Jun 1, 2023
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<optional>true</optional>
<scope>provided</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

This I don't fully understand, what if used outside of Maven, will this still work?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, just like any maven plugin (w/ these scopes) will not resolve all the needed dependencies outside of Maven. IMHO these artifacts are meant for one thing: to be dropped into ${maven.home}/lib/ext and nothing more. More cautiously one could say "to be dropped into existing (and working) resolver classpath", but then again scope is right, as "working classpath" of resolver HAS this artifact.

Copy link
Member Author

@cstamas cstamas Jun 1, 2023

Choose a reason for hiding this comment

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

One may consider these as "manually installed plugins for resolver" as that's what both do (hazelcast and redisson), they add themselves and their (not yet present) dependencies, like hazelcast and redisson w/ transitive hull.

Copy link
Member

Choose a reason for hiding this comment

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

But what is the pain then? You create a project with Resolver and Hazelcast named lock, all deps are there, not related to Maven usage. They end up in CP anyway, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, resolver brings in needed stuff, hence "provided"

@gnodet gnodet modified the milestones: 1.9.4, 1.9.11 Jun 1, 2023
@cstamas cstamas requested a review from michael-o June 1, 2023 14:15
@@ -39,21 +39,23 @@
<dependencies>
<dependency>
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-api</artifactId>
<artifactId>maven-resolver-named-locks</artifactId>
<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.

This chang will make dependency:analyze complain. You should reatin retain it at provided scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

No it will not, try it out

Copy link
Member Author

Choose a reason for hiding this comment

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

(but i ignore test scope in that output)

@cstamas cstamas merged commit 4bd99d8 into apache:master Jun 2, 2023
@cstamas cstamas deleted the update-dependencies branch June 2, 2023 08:30
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.

3 participants