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

[MPLUGIN-437] Fixes to the plugin descriptor generation #167

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Oct 28, 2022

When a mojo inherits from a mojo in a different project, the plugin descriptor generation fails.
Another problem is when the source artifact is being built in the same reactor, I had a case where the resolved artifacts file was pointing to a directory.

@gnodet gnodet changed the title Fixes Fixes to the plugin descriptor generation Oct 28, 2022
@slawekjaranowski
Copy link
Member

Can you create issue for it?

@gnodet gnodet changed the title Fixes to the plugin descriptor generation [MPLUGIN-437] Fixes to the plugin descriptor generation Oct 28, 2022
@gnodet gnodet force-pushed the plugin-generator-fixes branch from 1dca0a7 to c502601 Compare October 28, 2022 07:04
@gnodet gnodet merged commit ebdb063 into apache:master Oct 28, 2022
@@ -81,13 +80,6 @@ public JavaClassConverterContext( JavaClass mojoClass, JavaClass declaringClass,
Map<String, MojoAnnotatedClass> mojoAnnotatedClasses,
JavadocLinkGenerator linkGenerator, int lineNumber )
{
if ( !( mojoClass.getJavaClassLibrary() instanceof SourceLibrary )
Copy link
Member

Choose a reason for hiding this comment

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

why has this been removed? Access to import statements is still crucial for resolving javadoc references.

Copy link
Contributor Author

@gnodet gnodet Oct 28, 2022

Choose a reason for hiding this comment

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

I did remove the fact that the generation will throw an exception, not the fact that it will be used. Resolving javadoc reference is an order less important than having the plugin generate the descriptor. In my use case, it was throwing an exception and failed the whole build.

Copy link
Member

Choose a reason for hiding this comment

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

I understand this, but how can it work correctly? The exception was a good indicator to ease debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, in my case, the parent class was not part of the module being built, and the declaringClass.getJavaClassLibrary() is an instance of ClassLoaderLibrary.
The generation will still work, even if some links can not be resolved. But the plugin as a whole can not fail for this reason. And it's a valid use case, so even a warning may be too much, unless it can be disabled.

Copy link
Member

Choose a reason for hiding this comment

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

I think at least a warning is reasonable if javadoc links cannot be resolved.

Copy link
Member

Choose a reason for hiding this comment

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

Also how can meta information from the parent mojos be extracted which are not part of the binary (i.e. coming from javadoc like description, since or deprecated)? I think this requires evaluation of the parent's plugin descriptor....

Copy link
Contributor Author

@gnodet gnodet Oct 28, 2022

Choose a reason for hiding this comment

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

Also how can meta information from the parent mojos be extracted which are not part of the binary (i.e. coming from javadoc like description, since or deprecated)? I think this requires evaluation of the parent's plugin descriptor....

I guess they don't. But in my case, the plugin is used internally during the build, so the documentation is not much of a problem.

Instead of warning upfront, I think it would be better to display a warning at the time the information can not be retrieved. That would avoid irrelevant warnings to be displayed. For example if the parent class does not contain any parameter definition, a warning would be irrelevant noise.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, not sure how to accomplish this with QDox, though.

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