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

feat: support Maven 4.0.0-beta-4 #8

Closed
wants to merge 4 commits into from
Closed

feat: support Maven 4.0.0-beta-4 #8

wants to merge 4 commits into from

Conversation

tisonkun
Copy link
Owner

No description provided.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Copy link
Owner Author

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

@gnodet here is a fork to os-maven-plugin and I'm going to merge this and release 0.4.0 for the integration.

If you can take a look for review, it would be great.

This work is based on your patch trustin/os-maven-plugin#74

/**
* Describe why.
*/
public static void disable() {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This looks a bit hack. Perhaps we can add more explanation to it.

cc @gnodet

Signed-off-by: tison <wander4096@gmail.com>
try {
repoSessionProps.putAll(dict);
} catch (Exception ex2) {
// In Maven 4, DefaultCloseableSession uses an immutable map
Copy link
Owner Author

Choose a reason for hiding this comment

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

I test locally that only with this change (i.e., no DetectPropertyContributor and the disbale thing), it seems this plugin can work properly also.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it can be simplified. If needed, we may want to release a Maven 3 plugin and a Maven 4 plugin separately. I've raised #9, but feel free to cherry-pick to this PR (I could not push to it).

@tisonkun
Copy link
Owner Author

In favor of #9.

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