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

Add m2e lifecycle metadata for maven-dependency-plugin #1078

Conversation

kwin
Copy link
Member

@kwin kwin commented Nov 22, 2022

This closes #1077

@github-actions
Copy link

Test Results

606 tests  ±0   599 ✔️ ±0   8m 34s ⏱️ - 1m 15s
  97 suites ±0       7 💤 ±0 
  97 files   ±0       0 ±0 

Results for commit dbfe0c1. ± Comparison against base commit 9f3ad47.

@mickaelistria mickaelistria merged commit 791525d into eclipse-m2e:master Nov 22, 2022
@HannesWell
Copy link
Contributor

Thanks @kwin for this update. Your dedication to spread the BuildContextAPI is really helpful!

Do you plan to have a release of the maven-dependency-plugin soon?
Because otherwise users get warnings telling them they should update the dependency-plugin but that update is not available.
So if the 3.4 release will take some more time I suggest to at least remove the warning until then.

@kwin
Copy link
Member Author

kwin commented Nov 22, 2022

The release should happen soon: https://lists.apache.org/thread/xo0m4dztj0v67rdv4n843ls3dxny66xg

@HannesWell
Copy link
Contributor

Great. 👍🏽

@laeubi
Copy link
Member

laeubi commented Nov 23, 2022

I'm not sure if an error is appropriate here, why not a warning instead? I assume even with a release there will be some projects that do not update immediately and some older projects that probably never update so all will get an error marker now the user will not really be able to act upon.

@kwin
Copy link
Member Author

kwin commented Nov 23, 2022

The behaviour should be the same for all Maven plugins, and the existing ones emit an error already (https://github.com/kwin/m2e-core/blob/dbfe0c1fc2538694edc66a61fa404073b5cffd3b/org.eclipse.m2e.core/lifecycle-mapping-metadata.xml#L116). But I agree that most probably all should be changed to a warning.

@laeubi
Copy link
Member

laeubi commented Nov 23, 2022

A while back we changed missing mappings from error > warning so I think this would be applicable also.

Error seems really disturbing to the user given that the only harm that could happen is that nothing happens...

@HannesWell
Copy link
Contributor

Yes the behavior should be the same and yes warning should be enough.
But the enum PluginExecutionAction only has the values ignore, execute, configurator and error, which are effectively the supported children of an action element. So we have to add a warning element to support that.

The relevant code to adjust this is probably LifecycleMappingFactory.instantiateProjectConfigurators().

That being said, I did not manage to actually get the warning in the Editor. I only got Plugin execution not covered warnings...

@laeubi
Copy link
Member

laeubi commented Nov 23, 2022

I can't think of a use-case where error is anything useful, why should I error the user about my mojo in an IDE? So probably the "error" can just be treated as a warning marker...

@HannesWell HannesWell added this to the 2.1.0 milestone Nov 24, 2022
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.

Add m2e lifecycle metadata for maven-dependency-plugin 3.4.0+
4 participants