fix: format method types and table in Client Overview#2361
Conversation
|
|
| .append(" <tr>\n") | ||
| .append(" <td>") | ||
| .append(method.method) | ||
| .append(CommentFormatter.formatAsJavaDocComment(method.method, null)) |
There was a problem hiding this comment.
Let's see if we can refactor CommentFormatter so that it can be used for the whole table, otherwise this issue may happen again in the future. That being said, if it turns out to be more complicated that it supposed to, we can merge this for now and create another issue to track the refactoring effort.
There was a problem hiding this comment.
I investigated this a bit and I think it would be a little tricky to refactor CommentFormatter to be able to be applied to a whole table as I think it would need to have changes (or additions) to JavadocComment to only escape certain characters if they're not part of an html tag.
I could rewrite the table in general to use JavadocComment instead of StringBuilder so at least it would be explicitly clear when we are not applying standard formatting (e.g. escaping HTML characters). Let me know if you think that's worth doing or if I should create an issue for the larger refactoring effort.
suztomo
left a comment
There was a problem hiding this comment.
About the prevention of future problems, would you implement a test to verify the generated javadoc are good with regard to the Maven command in Java 17?
Are you thinking something like the downstream tests we configured in java-shared-config: https://github.com/googleapis/java-shared-config/pull/662/files and applied to sdk-platform-java? |
|
I didn't think of the downstream testing specifically. Yes, a check similar to the downstream check will address my ask. There are various ways to solve that. I'm asking "Next time when somebody breaks the javadoc generation logic, please block the pull request." |
Cool. I created an issue #2380 to track that work as separate from this PR. |

Fixes #2366 ☕️
Confirmed with java-logging that running
mvn javadoc:aggregatesuccessfully builds.