Skip to content

Conversation

@cstamas
Copy link
Member

@cstamas cstamas commented Dec 13, 2024

The beginning

https://asciinema.org/a/KxFoz2NUyI0joa4kM4ZRRCJgD

Changes:

  • (unrelated) contains fix for property handling in embedded executor and in lookup invoker
  • pulled -o (offline) option to "generic" Options from MavenOptions
  • introduce mvnsh (scripts, options, CLIng main class)
  • simplified invokers (only one context needed for maven now)
  • invokers made "reentrant", it all depends HOW context is created

Related PRs:


https://issues.apache.org/jira/browse/MNG-8437

The beginning
@cstamas cstamas requested a review from gnodet December 13, 2024 21:36
@cstamas cstamas self-assigned this Dec 13, 2024
@cstamas cstamas changed the title mvnsh [MNG-8437] mvnsh Dec 16, 2024
@cstamas cstamas marked this pull request as ready for review December 16, 2024 14:20
@cstamas cstamas added this to the 4.0.0-rc-3 milestone Dec 16, 2024
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

Awesome ! Really nice work !

@cstamas cstamas merged commit 49825e6 into apache:master Dec 17, 2024
13 checks passed
@cstamas cstamas deleted the mvnsh branch December 17, 2024 09:50
.build();

customizeSettingsRequest(context, settingsRequest);
if (context.eventSpyDispatcher != null) {
Copy link

Choose a reason for hiding this comment

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

Our regression tests fail because the SettingsBuilderRequest event no longer fires after this PR has been merged. Would that be because eventSpyDispatcher is now somehow null at this point?

Copy link
Member Author

Choose a reason for hiding this comment

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

This method is called twice, once (very early) when loading up extensions, then spy is null. The 2nd invocation is when "maven boots" and then spy should not be null...

Copy link

Choose a reason for hiding this comment

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

@jira-importer
Copy link

Resolve #9418

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