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

Enhancement for docprocessor #5028

Merged

Conversation

johnmarquez12
Copy link
Contributor

Current implementation of the DocProcessor provides access to types that extend the Element class such as TypeElement, ExecutableElement and VariableElement. However extracting javadoc from these is not possible without access to a DocTrees object, example shown in usages here. By providing access to the DocletEnvironment to the DocProcessor functions, we can get access to objects that can properly evaluate the javadoc of these elements.

@jansupol
Copy link
Contributor

jansupol commented Apr 5, 2022

This one is difficult.

On one hand, it breaks backward compatibility. It changes the DocProcessor interface which can be implemented by users, and the user implementation will stop working. I'd like to avoid this.

On the other hand, making the change backward compatible is challenging. Jersey made backward-incompatible changes in the past, but it is not ideal.

Will need to give this more thoughts

@jansupol jansupol requested a review from jbescos April 5, 2022 11:14
@jansupol
Copy link
Contributor

jansupol commented Apr 7, 2022

The issue can be with users implementing their own DocProcessor, which is being loaded by the ResourceDoclet. One option is to deprecate the methods in DocProcessor and create the default methods with the third arg there. The default methods just invoke the deprecated methods. The new implementation of course will override the default methods.

How ugly does that sound?

@johnmarquez12
Copy link
Contributor Author

johnmarquez12 commented Apr 7, 2022

The issue can be with users implementing their own DocProcessor, which is being loaded by the ResourceDoclet. One option is to deprecate the methods in DocProcessor and create the default methods with the third arg there. The default methods just invoke the deprecated methods. The new implementation of course will override the default methods.

How ugly does that sound?

Would deprecating the old methods and adding new ones with a third arg in DocProcessor, then calling the new methods along with the deprecated ones in ResourceDoclet be enough? This would at least give users time to move to the new methods. Sort of in line with what you suggested.

@jbescos
Copy link
Member

jbescos commented Apr 8, 2022

The issue can be with users implementing their own DocProcessor, which is being loaded by the ResourceDoclet. One option is to deprecate the methods in DocProcessor and create the default methods with the third arg there. The default methods just invoke the deprecated methods. The new implementation of course will override the default methods.

How ugly does that sound?

It looks ugly for a while, but eventually we can remove the deprecated methods and it will look pretty again.

I'm fine with this solution.

Copy link
Member

@jbescos jbescos left a comment

Choose a reason for hiding this comment

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

LGTM

@senivam senivam merged commit 08bfa50 into eclipse-ee4j:master Apr 25, 2022
@senivam senivam added this to the 2.36 milestone Apr 25, 2022
This was referenced Jun 14, 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.

None yet

4 participants