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

Issue20/add import resolver #46

Closed
wants to merge 2 commits into from

Conversation

kirkch
Copy link
Contributor

@kirkch kirkch commented Sep 19, 2020

This is a fairly large PR. To review, I suggest breaking it down into the following parts:

  1. addition of a new module for acceptance tests; only two tests at the moment that demonstrate this change.
  2. changes to scribe that adds imports to the output json.
  3. addition of ClassResolver (read in isolation using ClassResolverTest as a guide)
  4. and lastly, wiring the ClassResolver into the runtime javadoc parser

Please note that this PR's branches from branch #21, please review and merge that branch first. I did this so that this change would resolve @throws and @exception tags as well as @see.

Resolves #20.

@dnault
Copy link
Owner

dnault commented Oct 3, 2020

Hi Chris, thanks for this. Can you help me understand why the class resolution happens at runtime instead of at compile time?

@joffrey-bion
Copy link
Contributor

joffrey-bion commented Oct 3, 2020

Hi Chris, thanks for this. Can you help me understand why the class resolution happens at runtime instead of at compile time?

I was about to comment the same thing. I wonder why we don't just resolve the class names with respect to the imports at compile time, to provide the fully qualified class names in the JSON then. I think the notion of imports shouldn't make it to the JSON representation of the Javadoc.

@kirkch
Copy link
Contributor Author

kirkch commented Oct 3, 2020

Can you help me understand why the class resolution happens at runtime instead of at compile time?

I agree that having it done at the time of generating the json would be preferable. I flagged this before I started when I mentioned writing the imports to the json file. joffrey is right that not having them in there at all is preferable. At this stage, given how long the ticket has stood open, I would argue that a runtime resolver is preferable over having no resolver and this ticket remaining stagnant. Once we have how to resolve within the javac api licked, then the runtime resolver can be pulled. Or if this PR inspires somebody to write that now, that would be great.

@kirkch kirkch closed this Oct 9, 2020
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.

Provide the fully qualified class name in Link elements
3 participants